-
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
added tests for Fruit Shop #790
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.
First, not all checks passed.
The decision is very good!!! :) And if there is no desire to suffer - my comments are optional for you.
fix styleChecks (maybe a problem with the number of tests) or some small, isolated bugs
import org.junit.jupiter.api.BeforeAll; | ||
import org.junit.jupiter.api.Test; | ||
|
||
public class BalanceOperationHandlerTest { |
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.
what happens, when Quantity < 0 or new FruintTransaction has null as fruit name or null as operation type?
class PurchaseOperationHandlerTest { | ||
private static PurchaseOperationHandler purchaseOperationHandler; | ||
|
||
@BeforeAll |
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 would use here @beforeeach for init
I'm not sure if it will change much, so it`s up to u
int fruitsInStorage = Storage.STORAGE.get("banana"); | ||
assertEquals(30, fruitsInStorage); | ||
} | ||
|
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`d like to add test for negative quantity and null transaction (optional for u)
class ReturnOperationHandlerTest { | ||
private static ReturnOperationHandler returnOperationHandler; | ||
|
||
@BeforeAll |
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.
the same (BeforeEach) but we can wait what mentor says)
void tearDown() { | ||
Storage.STORAGE.clear(); | ||
} | ||
|
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.
we expect only a good result, mb will it be better to test notOk cases additionally?
void tearDown() { | ||
Storage.STORAGE.clear(); | ||
} | ||
|
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.
the same about notOk cases
class FileReaderServiceTest { | ||
private static FileReaderService fileReaderService; | ||
private static final String VALID_FILE_PATH = "src/test/java/resources/test.csv"; | ||
private static final String INVALID_FILE_PATH = "src/test/java/resources/InvalidTest.csv"; |
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.
Can we consider this as a wrong path, because there is a non-existent file, not a directory.
Perhaps this is a topic for conversation)
static void beforeAll() { | ||
fileWriterService = new FileWriterServiceImpl(); | ||
} | ||
|
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.
What happens if our report is null ?)
@@ -87,7 +87,7 @@ | |||
<limit> | |||
<counter>LINE</counter> | |||
<value>COVEREDRATIO</value> | |||
<minimum>0.8</minimum> | |||
<minimum>0.8 </minimum> |
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 u changed this ?
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 solution! Let's improve it)
} | ||
|
||
@Test | ||
void calculateOperation_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.
not informative name for method
FruitTransaction bananaTransaction = new FruitTransaction( | ||
FruitTransaction.Operation.BALANCE, "banana", -100 | ||
); | ||
assertThrows(RuntimeException.class, |
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 RuntimeException?
} | ||
|
||
@Test | ||
void processPurchaseOperation_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.
not informative name for method
FruitTransaction fruitTransaction = new FruitTransaction( | ||
FruitTransaction.Operation.PURCHASE, "banana", -100 | ||
); | ||
assertThrows(RuntimeException.class, |
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.
better to use InvalidDataException
FruitTransaction fruitTransaction = new FruitTransaction( | ||
FruitTransaction.Operation.PURCHASE, "banana", 70 | ||
); | ||
assertThrows(RuntimeException.class, |
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.
same
} | ||
|
||
@Test | ||
void processReturnOperation_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.
not informative name for method
} | ||
|
||
@Test | ||
void processSupply_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.
not informative name for method
FruitTransaction bananaTransaction = new FruitTransaction( | ||
FruitTransaction.Operation.BALANCE, "banana", -100 | ||
); | ||
assertThrows(RuntimeException.class, |
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.
same
FruitTransaction bananaTransaction = new FruitTransaction( | ||
FruitTransaction.Operation.BALANCE, "banana", -100 | ||
); | ||
assertThrows(RuntimeException.class, |
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.
same
if (transaction.getQuantity() < 0) { | ||
throw new InvalidDataException("Negative quantity"); | ||
} else if (transaction.getFruit() == null) { | ||
throw new NullPointerException("Invalid fruit type"); |
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.
throw new NullPointerException("Invalid fruit type"); | |
throw new InvalidDataException("Invalid fruit type"); |
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.
same everywhere
No description provided.