Tests #789

SalnikovMarkiian · 2023-09-19T20:15:23Z

No description provided.

Cayman4010

Oh, fantastic job! I didn`t think that we have to create so many tests... I am tired after reading this like after my work :)

Cayman4010 · 2023-09-20T08:14:53Z

src/test/java/core/basesyntax/services/FruitShopServiceImplTest.java

+
+    @AfterEach
+    public void tearDown() {
+        Storage.storage.clear();


I think double storage.clear is redundant (initStorage and tearDown), but I`m not sure.

Cayman4010 · 2023-09-20T08:21:39Z

src/test/java/core/basesyntax/services/ReportServiceImplTest.java

+import org.junit.jupiter.api.Test;
+
+public class ReportServiceImplTest {
+    private static final String LINE_SEPARATOR = System.lineSeparator();


It may be a redundant constant. Andrew told me about it in previous task.

i use LINE_SEPARATOR for resolve DRY problem, the constant helps not to repeat

DRY is not the case here - you don't have to use any constants to avoid DRY violation in your tests

read carefully what is DRY principle - multiple calls to System.lineSeparator() don't violate it

Cayman4010 · 2023-09-20T08:24:49Z

src/test/java/core/basesyntax/services/WriteServiceImplTest.java

+public class WriteServiceImplTest {
+    private static final String PATH_TO_FILE = "src/test/java/resources/report.csv";
+    private static final String INVALID_PATH = "invalid/path/to/file";
+    private static final String LINE_SEPARATOR = System.lineSeparator();


I still do not understand why constants do not have a good tone here

answered above

NazarSmith228

Good job! Let's improve your solution

NazarSmith228 · 2023-09-20T16:17:53Z

src/test/java/core/basesyntax/services/FruitShopServiceImplTest.java

+    private static final String PINEAPPLE = "pineapple";
+    private static final String BANANA = "banana";
+    private static final String ORANGE = "orange";
+    private static final String DRAGON_FRUIT = "dragonFruit";


constants are bad for tests - pls avoid them

NazarSmith228 · 2023-09-20T16:18:01Z

src/test/java/core/basesyntax/services/FruitShopServiceImplTest.java

+    public void setUp() {
+        initFruitShopService();
+        initStorage();
+


redundant blank line

NazarSmith228 · 2023-09-20T16:18:59Z

src/test/java/core/basesyntax/services/FruitShopServiceImplTest.java

+    @AfterEach
+    public void tearDown() {
+        Storage.storage.clear();


you can remove this hook, since you do the cleanup in @BeforeEach

NazarSmith228 · 2023-09-20T16:20:12Z

src/test/java/core/basesyntax/services/ParseServiceImplTest.java

+    private static final String HEAD_FILE = "operation,fruit,quantity";
+    private static final String INVALID_BALANCE_STRING = "b,apple,5A";
+    private static final String VALID_SUPPLY_ORANGE = "s,orange,5";


constants are bad for tests - pls avoid them

NazarSmith228 · 2023-09-20T16:25:30Z

src/test/java/core/basesyntax/services/FruitShopServiceImplTest.java

+        Storage.storage.put(PINEAPPLE, 10);
+        Storage.storage.put(BANANA, 20);
+        Storage.storage.put(ORANGE, 30);


it is not the best option to add some random data before every test since it complicates the reading process

for instance in one of your tests you have a FruitTransaction(Operation.PURCHASE, PINEAPPLE, 5), and then you do some assertion over pineapples. it's hard for me to understand the final value, since I always need to get back to @BeforeEach and check what exactly you stored before the test

NazarSmith228 · 2023-09-20T16:42:32Z

src/test/java/core/basesyntax/services/WriteServiceImplTest.java

+
+    @Test
+    public void writeToFile_Ok() throws IOException {
+        String expectedReport = "fruit,quantity" + LINE_SEPARATOR


can we use multi-line string here?

NazarSmith228 · 2023-09-20T16:51:09Z

src/test/java/core/basesyntax/services/WriteServiceImplTest.java

+
+    @Test
+    public void writeToFile_InvalidPath_notOk() {
+        String report = "fruit,quantity" + LINE_SEPARATOR


you can avoid real import here and pass some dummy string, since you expect exception at the end

NazarSmith228 · 2023-09-20T16:52:04Z

src/test/java/core/basesyntax/strategy/ReturnHandlerTest.java

+    @AfterEach
+    public void tearDown() {
+        Storage.storage.clear();
+    }


you can compose this in @BeforeEach

NazarSmith228 · 2023-09-20T16:52:30Z

src/test/java/core/basesyntax/strategy/BalanceHandlerTest.java

+    private static final String VALID_FRUIT_NAME = "dragonFruit";
+    private static final int INITIAL_BALANCE = 10;


constants are bad for tests - pls avoid them

NazarSmith228 · 2023-09-20T16:52:46Z

src/test/java/core/basesyntax/strategy/BalanceHandlerTest.java

+    @AfterEach
+    public void tearDown() {
+        Storage.storage.clear();
+    }


you can compose this in @BeforeEach

Elena-Bruyako · 2023-09-22T07:24:39Z

src/test/java/core/basesyntax/services/ParseServiceImplTest.java

+import org.junit.jupiter.api.Test;
+
+public class ParseServiceImplTest {
+


remove empty line

Elena-Bruyako · 2023-09-22T07:25:26Z

src/test/java/core/basesyntax/services/ParseServiceImplTest.java

+    }
+
+    @Test
+    public void test_ParseWithEmptyInput() {


is it ok case or notOk?

Elena-Bruyako · 2023-09-22T07:28:37Z

src/test/java/core/basesyntax/services/ReadServiceImplTest.java

+        List<String> expected = Files.readAllLines(Path.of(VALID_PATH_TO_VALID_FILE));
+        List<String> actual = readService.readFromFile(VALID_PATH_TO_VALID_FILE);
+        assertEquals(expected, actual);


Elena-Bruyako · 2023-09-22T07:28:45Z

src/test/java/core/basesyntax/services/ReportServiceImplTest.java

+import org.junit.jupiter.api.Test;
+
+public class ReportServiceImplTest {
+


remove empty line

Elena-Bruyako · 2023-09-22T07:33:15Z

src/test/java/core/basesyntax/strategy/PurchaseHandlerTest.java

+        purchaseHandler = new PurchaseHandler();
+    }
+
+    @AfterEach


Try to be consistent: in some test classes you clear the Storage by using AfterEach annotation, in some classes by using BeforeEach annotation

SalnikovMarkiian added 5 commits September 18, 2023 20:59

cop project, added test for BalanceHandler

32400b9

covered strategy impl by tests

e87525b

covered services impl by tests

64b31c7

fixed minor warnings & checkstyle rules

d0bec7a

minor fix checkstyle

6abaa0a

Cayman4010 approved these changes Sep 20, 2023

View reviewed changes

NazarSmith228 suggested changes Sep 20, 2023

View reviewed changes

fixed comments

84c0a2d

SalnikovMarkiian requested a review from NazarSmith228 September 21, 2023 19:36

Elena-Bruyako suggested changes Sep 22, 2023

View reviewed changes

resolved the specified problems

c4237e2

SalnikovMarkiian requested a review from Elena-Bruyako September 23, 2023 08:03

Elena-Bruyako approved these changes Sep 25, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tests #789

Tests #789

SalnikovMarkiian commented Sep 19, 2023

Cayman4010 left a comment

Cayman4010 Sep 20, 2023

Cayman4010 Sep 20, 2023

SalnikovMarkiian Sep 20, 2023

NazarSmith228 Sep 20, 2023

Cayman4010 Sep 20, 2023

SalnikovMarkiian Sep 20, 2023

NazarSmith228 Sep 20, 2023

NazarSmith228 left a comment

NazarSmith228 Sep 20, 2023

NazarSmith228 Sep 20, 2023

NazarSmith228 Sep 20, 2023

NazarSmith228 Sep 20, 2023

NazarSmith228 Sep 20, 2023

NazarSmith228 Sep 20, 2023

NazarSmith228 Sep 20, 2023

NazarSmith228 Sep 20, 2023

NazarSmith228 Sep 20, 2023

NazarSmith228 Sep 20, 2023

Elena-Bruyako Sep 22, 2023

Elena-Bruyako Sep 22, 2023

Elena-Bruyako Sep 22, 2023

Elena-Bruyako Sep 22, 2023

Elena-Bruyako Sep 22, 2023

		private static final String VALID_FRUIT_NAME = "dragonFruit";
		private static final int INITIAL_BALANCE = 10;

		import org.junit.jupiter.api.Test;

		public class ParseServiceImplTest {

		import org.junit.jupiter.api.Test;

		public class ReportServiceImplTest {

Tests #789

Are you sure you want to change the base?

Tests #789

Conversation

SalnikovMarkiian commented Sep 19, 2023

Cayman4010 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NazarSmith228 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment