• Najnowsze pytania
  • Bez odpowiedzi
  • Zadaj pytanie
  • Kategorie
  • Tagi
  • Zdobyte punkty
  • Ekipa ninja
  • IRC
  • FAQ
  • Regulamin
  • Książki warte uwagi

Testy jednostkowe metod z logerami

VPS Starter Arubacloud
0 głosów
492 wizyt
pytanie zadane 19 sierpnia 2018 w Java przez must Bywalec (2,980 p.)
edycja 19 sierpnia 2018 przez must

Cześć. Przychodze do Was z kolejnym problemem odnośnie testów. Czytałem już masę wątków odnośnie testowania loggerów. Np ten: https://stackoverflow.com/questions/1827677/how-to-do-a-junit-assert-on-a-message-in-a-logger . Mimo to, dalej ciężko mi napisac jakikolwiek test z tego względu, że nie wiem co dokładnie ma się w nim znajdować. Z tego co wyczytałem, trzeba stworzyć swojego włansego Appendera i ciężko będzie użyć tutaj mokowania.

Metoda, którą chcę przetestować:

void getAllCustomers() throws SQLException {
    for (int i = 0; i < storage.getAllCustomers().size(); i++) {
        logger.info("Name: " + storage.getAllCustomers().get(i).getName()
                + "\nSurname: " + storage.getAllCustomers().get(i).getSurname()
                + "\nStreet: " + storage.getAllCustomers().get(i).getStreet()
                + "\nHouse number: " + storage.getAllCustomers().get(i).getHouseNumber()
                + "\nCity: " + storage.getAllCustomers().get(i).getCity()
                + "\nPesel Number: " + storage.getAllCustomers().get(i).getPeselNumber()
                + "\nRent Date: " + storage.getAllCustomers().get(i).getRentDate()
                + "\nClient number: " + storage.getAllCustomers().get(i).getClientNumber());
        logger.info("---------------------------");
    }
}

Metoda storage.getAllCustomers():

@Override
public List<Client> getAllCustomers() throws SQLException {
    List<Client> listOfClients = new ArrayList<Client>();

    String sql = "SELECT * FROM `client`";
    result = statement.executeQuery(sql);

    while (result.next()) {
        Client client = new Client();
        client.setName(result.getString("namee"));
        client.setSurname(result.getString("surname"));
        client.setStreet(result.getString("street"));
        client.setPeselNumber(result.getLong("peselNumber"));
        client.setRentDate(result.getString("rentDate"));
        client.setCity(result.getString("city"));
        client.setHouseNumber(result.getInt("houseNumber"));
        client.setClientNumber(result.getInt("clientNumber"));

        listOfClients.add(client);
    }

    return listOfClients;
}

W klasie z metodą, którą chcę przetestować używam:

 private CarRentalStorage storage;
 private Logger logger = LoggerFactory.getLogger(CarRentalOptions.class);
komentarz 19 sierpnia 2018 przez RafalS VIP (122,820 p.)
Zacząłbym od tego, że jest to napisane maksymalnie niewydajnie :D. Za każdym razem gdy chcesz cokolwiek ze storage pobierane są z bazy danych wszystkie dane, jest tworzona ich lista i wszystko po to, żeby wyciągnać pojedyńczą wartość rekordu z tej listy, która jest zapominana w kolejnej linijce i znów pobieramy wszystkie dane z bazy i tworzymy nową liste. Wszystko powtarzane 10 razy w pętli :O Żeby pobrać dane o jednym customerze 10 razy osobno pobierane są dane o wszystkich customerach i jest tworzone 10 list wszystkich customerów :O
komentarz 19 sierpnia 2018 przez must Bywalec (2,980 p.)

A nie jest dodawany obiekt do tej samej listy?

listOfClients.add(client);

Jeżeli nie, to jak to efektywnie przerobić?

komentarz 19 sierpnia 2018 przez RafalS VIP (122,820 p.)
edycja 19 sierpnia 2018 przez RafalS

Każde takie wywołanie:

storage.getAllCustomers()

tworzy nową listę wszystkich customerów. Wywołujesz tą funkcję 10 razy w jednej pętli totalnie niepotrzebnie. Stwórz jedną listę na początku i z niej korzystaj:

List<Customer> customers = storage.getAllCustomers();

korzystaj z customers a nie bombarduj bazy niepotrzebnymi zapytaniami i programu niepotrzebnym tworzeniem i niszczeniem list.

Ja bym to widział tak, że klasa Customer potrafi skonwertować się do stringa, czyli ma nadpisaną metode toString wtedy wypisanie wyglądało by tak:

    void getAllCustomers() throws SQLException {
        List<Customer> customers = storage.getAllCustomers();
        customers.forEach(logger::info);
    }

Alternatywnie są też takie opcje, jesli nie lubisz referencji do funkcji:

        customers.forEach(customer -> logger.info(customer)); //lambda
        for(Customer customer : customers){ //zwykla petla
            logger.info(customer);
        }

i nazwa jest bez sensu. GetAllCustomers, które nic nie zwraca? Jak get to get.

 

komentarz 19 sierpnia 2018 przez RafalS VIP (122,820 p.)
Druga sprawa jest taka, że metoda getAllCustomers loguje customerów. Albo metoda się źle nazywa albo to nie ma sensu.
komentarz 19 sierpnia 2018 przez RafalS VIP (122,820 p.)
Trzecia sprawa jest taka, że pierwszy raz spotykam się z testowaniem logów :D Nie wiem za co ma być odpowiedzialna klasa, która loguje bo nie widze nawet jej nazwy, ale jeśli jej funkcjonalnością jest logowanie customerów to ja czegoś nie rozumiem :P Klasa pobiera dane z bazy i dodaje je do innej "bazy" logów?
komentarz 19 sierpnia 2018 przez must Bywalec (2,980 p.)
edycja 19 sierpnia 2018 przez must
Ah.. Teraz rozumiem. Pobieramy jedną wartość, tworzymy listę, wyciągamy wartość za pomocą get(i) i tak 10 razy na jednego użytkownika.

Jeżeli chodzi o nazwę logger, to wziąłme ją stąd, ze w sumie wszyscy ją tak nazywają. Mógłbym ja równoznacznie nazwac printer.

Korzystałem z wielu linków podobnych do tego https://stackoverflow.com/questions/2727500/log4j-vs-system-out-println-logger-advantages

Podobno lepiej jest używać loggerów niż sys.out.println z wielu wymienonych w linku i na innych stronach powodów m.in są szybsze od podstawowego wypisania sys.out

Metoda ma wyciągnąc wszystkich klientów z bazy i wypisać je na ekran. Taki był zamysł i przeznaczenie tej metody.

1 odpowiedź

0 głosów
odpowiedź 20 sierpnia 2018 przez must Bywalec (2,980 p.)

Metoda, którą chce przetestować zgodnie z @RafalS radami wygląda tak:

void printPersonalDataOfClients() throws SQLException {
    List<Client> listOfClients;
    listOfClients = storage.getAllCustomers();

    for (int i = 0; i < listOfClients.size(); i++)
        logger.info(String.valueOf(listOfClients.get(i)));

}

 Moglibyście mi coś doradzić odnośnie tych testów?

komentarz 20 sierpnia 2018 przez mbabane Szeryf (79,280 p.)

Może zrób tak, żeby ta metoda zwracała String i sprawdź czy jest w nim to czego oczekujesz:

String printPersonalDataOfClients() throws SQLException {
    List<Client> listOfClients;
    listOfClients = storage.getAllCustomers();

   StringBuilder sb = new StringBuilder();
   for (int i = 0; i < listOfClients.size(); i++)
        sb.append(String.valueOf(listOfClients.get(i)));

  return sb.toString();
}

 

komentarz 20 sierpnia 2018 przez RafalS VIP (122,820 p.)

@must, szczerze to nie wiem po co tu coś testować :D.

Za poprawność danych odpowiada klasa obslugujaca baze danych. Tutaj to możesz jedynie zamockować storage i porównać stringi. Dla mnie to jak testowanie getterów :P

komentarz 20 sierpnia 2018 przez must Bywalec (2,980 p.)

@mbabane, pod jakim wzgledem Twoje rozwiązanie jest lepsze?

 

@RafalS, mam takie 3 metody, które odpowiadają za wystwietlanie danych za pomocą loggerow. Wg Ciebie całkowicie pominąć ich testowanie? Obniża to co prawda Coverage, chciałem szczerze powiedziawszy przetestować je 3 i skonczyc swoje testy. Rzuciłbys okiem na moja projekt na githubie i powiedział co ewentualnie przetestować jeszcze? Bo sam już nie wiem:D

1
komentarz 20 sierpnia 2018 przez mbabane Szeryf (79,280 p.)
Czy lepsze to tego do końca nie wiem. Wydaje mi się że kiedy metoda nic nie zwraca to jest trochę kłopotliwa w testach. Zmieniając metodę na to że zwraca String można łatwo sprawdzić w testach czy komunikat spełnia Twoje założenia, bez zakładania mocka na Logger.
komentarz 21 sierpnia 2018 przez must Bywalec (2,980 p.)

Okej. Niby nie ma tego loggera, ale teraz zaminiam listę w Stringa, co jest troche dziwne.

Gdyby metoda wyglądała tak jak mówisz, czyli:

String getAllClients() throws SQLException {
    List<Client> listOfClients = storage.getAllCustomers();
    StringBuilder sb = new StringBuilder();

    for (Client clientPersonalData : listOfClients)
        sb.append(String.valueOf(clientPersonalData));

    return sb.toString();
}

To i tak mi jest to ciężko przetestować,z  tego względu, ze nie przekazuje tutaj żadnego inputa, w wyniku czego nie moge przewidzieć jaki będzie output.

komentarz 21 sierpnia 2018 przez mbabane Szeryf (79,280 p.)
To stwórz (zmockuj) storage testowy, w którym dane będą kontrolowane przez Ciebie.
komentarz 21 sierpnia 2018 przez must Bywalec (2,980 p.)

@mbabane, mam stworzyć jakąś klasę, która będzie zwracała gotowy tekst, bo nie rozumiem?

komentarz 21 sierpnia 2018 przez mbabane Szeryf (79,280 p.)
Wiesz czym jest mockowanie?
komentarz 21 sierpnia 2018 przez must Bywalec (2,980 p.)

Tak wiem.

Chodzi Ci o coś takiego:

void getAllClients() throws SQLException {
    carRentalOptions.getAllClients();
    
    verify(carRentalStorageMock).getAllCustomers();
}

?

komentarz 21 sierpnia 2018 przez mbabane Szeryf (79,280 p.)

Chodzi o to żeby zmockować ten storage dzięki temu możesz skontrolować czy printPersonalDataOfClients generuje odpowiedni komunikat.

W Mockito zdaje się że robiło się to jakoś tak:

List<Client> clients = new ArrayList<>();
clients.add(new Client() );

when( carRentalStorageMock.getAllCustomers() ).thenReturn( clients );

Dzięki temu getAllCustomers() zwraca stałą wartość i umożliwia skontrolowanie metody np. printPersonalDataOfClients .

komentarz 21 sierpnia 2018 przez must Bywalec (2,980 p.)

No dobrze, ale co po tym

carRentalOptions.getAllClients();

?

 

Porównać się nie da, bo porównuje listę oraz String.

komentarz 21 sierpnia 2018 przez mbabane Szeryf (79,280 p.)
edycja 21 sierpnia 2018 przez mbabane

Wydaje mi się, że nie do końca rozumiesz po co jest Mockito. 

Zerknij na przykład:

Prosta encja:

public class Client
{
    private String name;
    private String lastName;

    public Client(String name, String lastName)
    {
        this.name = name;
        this.lastName = lastName;
    }

    @Override
    public String toString()
    {
        return "Client{" +
                "name='" + name + '\'' +
                ", lastName='" + lastName + '\'' +
                '}';
    }
}

Storage jako interfejs bo interesuje nas zachowanie, nie implementacja. W testach ma to szczególne znaczenie, bo dzięki temu pod Storage można podpiąć np. bazę danych w ramie czy nawet zaimplementować to jako prostą klasę używaną tylko w testach:

public interface Storage
{
    public List<Client> getAllClients();
}

Jakiś tam serwis korzystający ze Storage. Storage przekazywany przez konstruktor dzięki temu łatwo jest podmienić jego implementację:

public class Service
{
    private Storage storage;

    public Service(Storage storage)
    {
        this.storage = storage;
    }

    public String printClients()
    {
        List<Client> clients = storage.getAllClients();

        StringBuilder sb = new StringBuilder();

        for(Client client : clients)
            sb.append(String.valueOf(client));

        return sb.toString();
    }
}

I teraz test jednostkowy, z wykorzystaniem Mockito dla Storage:

public class ServiceTest
{
    @Test
    public void someUnitTest()
    {
        List<Client> clients = new ArrayList<>();
        clients.add(new Client("Ferdynand", "Kiepski"));
        clients.add(new Client("Marian", "Paździoch"));

        Storage storageMock = Mockito.mock(Storage.class);
        Mockito.when(storageMock.getAllClients())
               .thenReturn( clients );

        String expected = "Client{name='Ferdynand', lastName='Kiepski'}Client{name='Marian', lastName='Paździoch'}";
        
        Service service = new Service(storageMock);
        String actual = service.printClients();
        System.out.println(actual);

        Assert.assertEquals(expected, actual);
    }
}
1
komentarz 21 sierpnia 2018 przez must Bywalec (2,980 p.)

To prawda. Nie rozumiem do końca tematu mocków, czytałem mase już stron na ten temat, jednakże na ten moment nie znalazłem tej "jedynej".

Do rzeczy.

Wszystko rozumiem poza:

Mockito.when(storageMock.getAllClients()).thenReturn( clients );

Po co to jest dokładnie? Czy jest to przez to, że w metodzie którą testujemy jest wywołanie .getAllClients() ? Bo w samym tescie tego nie używamy. Czytam na internecie, ze:

The thenReturn() methods lets you define the return value when a particular method of the mocked object is been called.

 

Pytam z tego powodu, bo wracając do mojego poprzedniego wątku:

https://forum.pasja-informatyki.pl/372673/dlaczego-po-dodaniu-adnotacji-%40mock-wywala-mi-java-lang-nullpointerexception?show=373280#a373280

Tutaj nie jest w ogole potrzebne when().... myli mi się to masakrycznie.

 

komentarz 21 sierpnia 2018 przez must Bywalec (2,980 p.)

Test nie działa. Dzieje się to co w poprzednim wątku. 

java.lang.NullPointerException
	at car.rental.CarRentalOptionsTest.getAllClients(CarRentalOptionsTest.java:96)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:564)
	at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:515)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:115)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$6(TestMethodTestDescriptor.java:170)
	at org.junit.jupiter.engine.execution.ThrowableCollector.execute(ThrowableCollector.java:40)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:166)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:113)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:58)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:134)
	at org.junit.platform.engine.support.hierarchical.SingleTestExecutor.executeSafely(SingleTestExecutor.java:66)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:128)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:109)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1378)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.SingleTestExecutor.executeSafely(SingleTestExecutor.java:66)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:128)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:109)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1378)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.SingleTestExecutor.executeSafely(SingleTestExecutor.java:66)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:128)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:109)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:32)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:49)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:47)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:184)
	at org.junit.platform.launcher.core.DefaultLauncher.lambda$execute$5(DefaultLauncher.java:152)
	at org.junit.platform.launcher.core.DefaultLauncher.withInterceptedStreams(DefaultLauncher.java:166)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:145)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:92)
	at com.intellij.junit5.JUnit5IdeaTestRunner.startRunnerWithArgs(JUnit5IdeaTestRunner.java:74)
	at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
	at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
	at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)

Chodzi o tę linijkę: 

when(carRentalStorageMock.getAllCustomers()).thenReturn(listOfClients);

Klasa wygląda tak:

@Mock
private CarRentalStorage carRentalStorageMock;
private CarRentalOptions carRentalOptions;
private Client client;

@Before
public void setup() {
    MockitoAnnotations.initMocks(this);
    carRentalOptions = new CarRentalOptions(carRentalStorageMock);
}
@org.junit.jupiter.api.Test
void getAllClients() throws SQLException {
    client = new Client();
    List<Client> listOfClients = new ArrayList<Client>();
    listOfClients.add(client);


    when(carRentalStorageMock.getAllCustomers()).thenReturn(listOfClients);

    String expected = "Client{name='Ferdynand', lastName='Kiepski'}Client{name='Marian', lastName='Paździoch'}";


    String actual = carRentalOptions.printClients();
    System.out.println(actual);

    assertEquals(expected, actual);
}

 

komentarz 21 sierpnia 2018 przez mbabane Szeryf (79,280 p.)
Mockito.when(storageMock.getAllClients()).thenReturn( clients );

Można to przetłumaczyć tak:

kiedy na obiektcie storageMock zostanie wywołana metoda getAllClients(), wtedy zwróć obiekt clients.

 Czy jest to przez to, że w metodzie którą testujemy jest wywołanie .getAllClients() 

Zgadza się. W teście interesuje Cię metoda printClients (czy jak tam ją nazwałeś), ale korzysta ona z dodatkowej zależności w postaci Storage. I tutaj z pomocą przychodzi Mockito, dzięki któremu kontrolowany jest obiekt Storage, tak aby na pewno nie zaburzał pracy printClients, co pozwala sprawdzić czy metoda robi to co ma robić.

komentarz 21 sierpnia 2018 przez must Bywalec (2,980 p.)

Dobrze. Ale w metodzie, którą testowałem w poprzednim wątku:

Założmy ta:

void createNewCar(Car car) throws SQLException {
    storage.addNewCar(car);
}

Metoda tak samo korzysta z bazy danych, ale tutaj when nie było potrzebne i test wygląda tak:

@Test
public void createNewCar() throws SQLException {
    car = new Car();
    carRentalOptions.createNewCar(car);

    verify(carRentalStorageMock).addNewCar(car);
}
1
komentarz 21 sierpnia 2018 przez mbabane Szeryf (79,280 p.)
Ten test sprawdza Ci czy na obiekcie storage została wywołana metoda addNewCar, więc manipulacja zachowaniem metody addNewCar zdaje się nie jest potrzebna. Pamiętaj to są narzędzia, to że w skrzynce jest młotek nie znaczy że trzeba z niego korzystać przy każdej pracy.
komentarz 21 sierpnia 2018 przez must Bywalec (2,980 p.)
Tam u góry pisałem o NPE.

Spowodowane było to tym, że musiałem wszystkie dane ustawić klienta ;). Także to rozwiązałem.
komentarz 21 sierpnia 2018 przez must Bywalec (2,980 p.)
Swoją drogą, nazwa printClients chyba jest tutaj błędna i powinieniem wrócić do poprzedniej getPersonalDataofClients, bo printClients w sumie nic nie wypisuje, a zwraca. Prawda?
komentarz 21 sierpnia 2018 przez mbabane Szeryf (79,280 p.)
Rzeczywiście printClients może wprowadzać w błąd. getPersonalDataofClients w sumie też troche. Ta druga nazwa może sugerować, że zwraca jakąś listę obiektów (listę w sensie List<E>). Jeśli to ma być informacja przekazana do loggera, z którego potem korzysta programista to nazwa tej metody może być np. debugClients.
komentarz 21 sierpnia 2018 przez must Bywalec (2,980 p.)
Tak, przekazywane jest to później do loggera. Wszystko po to, by tylko wypisać to w konsoli, Jest to aplikacja konsolowa. Czytałem na internecie, ze lepiej tego używać anizeli sys.out. Stąd ten logger.
1
komentarz 21 sierpnia 2018 przez mbabane Szeryf (79,280 p.)
To może getFullInfoAboutClients
komentarz 2 września 2018 przez must Bywalec (2,980 p.)

Hej, mam podobną metodę do przetestowania:

public List<Task> getTasks() throws SQLException {
    return taskStorage.getTasks(connectedUser);
}

Testuję ją tak:

@Mock
TaskInterface taskStorageMock;
@Mock
UserInterface userStorageMock;
private ToDoEngine toDoEngine;
@Before
public void setup() {
    MockitoAnnotations.initMocks(this);
    toDoEngine = new ToDoEngine(userStorageMock, taskStorageMock);
}
@Test
public void getTasks() throws SQLException {
    List<Task> tasks = new ArrayList<Task>();
    User user = new User("admin","123");
    Task task = new Task("wash");
    tasks.add(task);
    when(taskStorageMock.getTasks(user)).thenReturn(tasks);

    List<Task> actual = toDoEngine.getTasks();
    List<Task> expected = taskStorageMock.getTasks(user);

    assertEquals(expected,actual);

}

Czyli bardzo podobnie, wywala jednak takie cos:

java.lang.AssertionError: 
Expected :[Task: wash]
Actual   :[]
 <Click to see difference>


	at org.junit.Assert.fail(Assert.java:88)
	at org.junit.Assert.failNotEquals(Assert.java:834)
	at org.junit.Assert.assertEquals(Assert.java:118)
	at org.junit.Assert.assertEquals(Assert.java:144)
	at controller.ToDoEngineTest.getTasks(ToDoEngineTest.java:54)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:564)
	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.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.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
	at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
	at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
	at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)

 

komentarz 2 września 2018 przez mbabane Szeryf (79,280 p.)

A jak wyznaczane jest connectedUser w:

public List<Task> getTasks() throws SQLException {
    return taskStorage.getTasks(connectedUser);
}

I czy w sumie nie jest trochę bez sensu to:

List<Task> expected = taskStorageMock.getTasks(user);

Skoro kilka linijek wyżej masz zdaje się to samo:

  List<Task> tasks = new ArrayList<Task>();
    User user = new User("admin","123");
    Task task = new Task("wash");
    tasks.add(task);

Żeby to zadziałało:

when(taskStorageMock.getTasks(user)).thenReturn(tasks);

to zdaje się że w metodzie getTasks musi pojawić się dokładnie to samo co w when więc może spróbuj np. tak:

when( taskStorageMock.getTasks( any() ) ).thenReturn(tasks);
komentarz 2 września 2018 przez must Bywalec (2,980 p.)

Tak jest wyznaczane:

public boolean signIn(String username, String password) throws SQLException {
    connectedUser = new User(username, password);
    if (!userStorage.signIn(connectedUser))
        return false;
    connectedUser.setID(retrieveConnectedUserID(connectedUser));
    return true;
}

Faktycznie działa, jeżeli zrobie cast na Usera czyli (User)any()

Ale jak to jest to samo tutaj: 

List<Task> expected = taskStorageMock.getTasks(user);

Nie rozumiem.

 

Chore te testy są dla mnie:D Ktoś pisał, że poprawiają kodowanie, ale 1/5 testów potrafie sam zrobić w wyniku czego stoje cały czas w miejscu, bo sie głowie co poprawic.

komentarz 2 września 2018 przez mbabane Szeryf (79,280 p.)

Szczerze powiedziawszy to do kitu jest to wyznaczanie usera. Dlaczego nie zrobisz chociażby tak:

connectedUser = userStorage.getUSerByNameAndPass(name, password);

a w Twoim sposobie instancje usera i tak masz bo robisz:

connectedUser = new User(username, password);

Pobieraj go bezpośrednio z bazy. Nie ma go w bazie to nie ma instancji. UserStorage również należałoby w takim wypadku zmockować tak żeby zwracał Ci: 

User user = new User("admin","123");

 

Ale jak to jest to samo tutaj: 

Bo tu ustawiasz że ma być to zwrócone własnie przez taskStorageMock:

when(taskStorageMock.getTasks(user)).thenReturn(tasks);

 

A z tym any() żeby było ładniej to zrób coś takiego:

any(User.class)

Testy ułatwiają prace tylko w wielkich projektach, i do których wprowadzane są ciągłe zmiany. Typowo szkolno-domowe projekty raczej do takich nie należą i testy w nich nie mają tak wielkiego znaczenia. Jednak, jak wszystko, należy je jakoś trenować.

komentarz 2 września 2018 przez must Bywalec (2,980 p.)

Dlaczego nie zrobisz chociażby tak

Bo metoda odpowiedzialna za logowanie wygląda tak:

public boolean signIn(User user) throws SQLException {
        PreparedStatement preparedStatement = connection.prepareStatement("SELECT COUNT(username) FROM user WHERE username=? AND password=? LIMIT 0,1");
        preparedStatement.setString(1, user.getName());
        preparedStatement.setString(2, user.getPassword());
        ResultSet result = preparedStatement.executeQuery();
        int count = 0;
        if (result.next())
            count = result.getInt(1);
         if (count < 1)
            return false;
        return true;
    }

Bo tu ustawiasz że ma być to zwrócone własnie przez taskStorageMock:

No tak, tutaj:

when(taskStorageMock.getTasks(user)).thenReturn(tasks);

mówie, ze gdy bedzie wywołane getTasks to ma mi zwrócić listę, a gdzie tutaj o tym mówię:

List<Task> tasks = new ArrayList<Task>();
  User user = new User("admin","123");
  Task task = new Task("wash");
  tasks.add(task);

jak tutaj tworze dopiero listę tasków.

komentarz 3 września 2018 przez mbabane Szeryf (79,280 p.)

W sumie to czemu tak:

public boolean signIn(User user) throws SQLException {
        PreparedStatement preparedStatement = connection.prepareStatement("SELECT COUNT(username) FROM user WHERE username=? AND password=? LIMIT 0,1");
        preparedStatement.setString(1, user.getName());
        preparedStatement.setString(2, user.getPassword());
        ResultSet result = preparedStatement.executeQuery();
        int count = 0;
        if (result.next())
            count = result.getInt(1);
         if (count < 1)
            return false;
        return true;
    }

Skoro każdy user ma być unikalny przez login, to dlaczego od razu nie weźmiesz z bazy tego co potrzeba? Jak tego nie będzie to ResultSet będzie pusty. Rozumiem, że po powyższym robisz kolejne połącznie do bazy, które pobiera dane o użytkowniku?

 

mówie, ze gdy bedzie wywołane getTasks to ma mi zwrócić listę, a gdzie tutaj o tym mówię:

Nie o to chodzi. Chodzi o to, że pobierasz listę z taskStorageMock, którą już posiadasz tutaj:

List<Task> tasks = new ArrayList<Task>();
  User user = new User("admin","123");
  Task task = new Task("wash");
  tasks.add(task);

To nie jest błąd testu tylko taki bardziej logiczny. No chyba, że zrobiłeś to żeby się przekonać czy rzeczywiście mockowanie zadziałało.

komentarz 3 września 2018 przez must Bywalec (2,980 p.)
Szukam w bazie po loginie i haśle, jak jest taki to zwracam true, jak nie to false.

Mam rozumieć, ze chcesz bym tutaj zwrócił obiekt usera? A co jak go nie będzie?
komentarz 3 września 2018 przez mbabane Szeryf (79,280 p.)

Mam rozumieć, ze chcesz bym tutaj zwrócił obiekt usera? A co jak go nie będzie?

Tak, bo tak jak masz to zdaje się że pobierasz go dwukrotnie prawda? Najpierw sprawdzasz czy jest, zliczając countem rekordy, a potem jeśli będzie 1 rekord to pobierasz faktyczne dane, tak?.

 

A jeśli go nie będzie to ResultSet.next() zwróci false. 

komentarz 3 września 2018 przez must Bywalec (2,980 p.)
Nie. Program szuka w bazie i zlicza ilość wystąpień tego nicku i passwordu, nie pobiera go z bazy, tylko zwraca true albo fakse.

Jak zwrócić obiekt usera skoro metoda jest typu boolean. Chyba, ze mam przerobic na typ User i zwraca albo null jak nie ma albo usera.
komentarz 3 września 2018 przez mbabane Szeryf (79,280 p.)
Tak chyba będzie najwygodniej. Można by jeszcze spróbować wykorzystać do tego typ Optional<T> żeby nie bawić się w nulle
komentarz 3 września 2018 przez must Bywalec (2,980 p.)

Zedytowałem tam u góry odpowiedź.

Nie rozumiem dlaczego mój sposób jest zły, tym bardziej, ze jak teraz zmienie na typ zwracany user, to będzie trzeba przerabiać wszystkie inne metody.

Choć nie, nie musiałbym przerabiać bo mógłbym zrobić cos takiego:

metoda(..)
if(connectedUser==null)
return false
else
return true

czyli praktycznie zadna zmiana.

1
komentarz 3 września 2018 przez mbabane Szeryf (79,280 p.)

Ja np. robiłbym to mniej więcej tak:

public class Sometests
{
    @Test
    public void unitTest() throws Exception
    {
        Optional<User> userOptional = getUserByUsernameAndPassword("ferd", "1234");
        try
        {
            User user = userOptional.orElseThrow(() -> new Exception("Błedny login lub hasło"));
            System.out.println(user);
        }
        catch (Exception e)
        {
            System.out.println(e.getMessage());
        }
    }

    public Optional<User> getUserByUsernameAndPassword(String username, String password) throws Exception
    {
        Connection connection = DriverManager.getConnection(DB_URL, DB_USR, DB_PASS);
        PreparedStatement statement = connection.prepareStatement("SELECT id, username FROM users WHERE username=? AND password=?");

        statement.setString(1, username);
        statement.setString(2, password);

        try (ResultSet resultSet = statement.executeQuery())
        {
            if (resultSet.next())
            {
                return Optional.of(new User(resultSet.getInt(1), resultSet.getString(2)));
            }
            return Optional.empty();
        }     
    }
}

Optional ułatwia sytuację z nullami tzn. jest bardziej czytelnie niż pisanie co chwila if (object == null). I w razie gdy nie ma usera w bazie to zwracam pustego Optionala, a w metodzie gdzie używam getUserByUsernameAndPassword za pomocą orElseThrow rzucam wyjątek z informacją.

komentarz 3 września 2018 przez must Bywalec (2,980 p.)
Tylko teraz w czym to jest lepsze?
komentarz 3 września 2018 przez mbabane Szeryf (79,280 p.)
Jest wydaje mi się dużo czytelniejsze i prostsze, i nie wymaga używania co chwila if'a żeby sprawdzać czy zwrócono true czy false. Dodatkowo masz od razu obiekt User, plus o jedno mniej połączeń do bazy (o ile robisz to tak jak myślę bo w sumie do końca nie napisałeś).

Za plus można też uznać ze korzysta się z Javy 8.
komentarz 3 września 2018 przez mbabane Szeryf (79,280 p.)
Jeszcze tak napiszę. Zastosuj tę metodę, którą lepiej Ty rozumiesz - to będzie wówczas najlepszy dla Ciebie sposób, bo będziesz potrafił go omówić i uzasadnić.
komentarz 3 września 2018 przez must Bywalec (2,980 p.)

Dzięki @mbabane:)

Podobne pytania

0 głosów
0 odpowiedzi 214 wizyt
pytanie zadane 8 lipca 2017 w Java przez Sidzej Użytkownik (850 p.)
0 głosów
1 odpowiedź 157 wizyt
pytanie zadane 14 października 2019 w Java przez heartagram Obywatel (1,770 p.)
0 głosów
1 odpowiedź 140 wizyt
pytanie zadane 3 sierpnia 2018 w Java przez kamil159 Nowicjusz (180 p.)

92,453 zapytań

141,262 odpowiedzi

319,087 komentarzy

61,854 pasjonatów

Motyw:

Akcja Pajacyk

Pajacyk od wielu lat dożywia dzieci. Pomóż klikając w zielony brzuszek na stronie. Dziękujemy! ♡

Oto polecana książka warta uwagi.
Pełną listę książek znajdziesz tutaj.

Akademia Sekuraka

Akademia Sekuraka 2024 zapewnia dostęp do minimum 15 szkoleń online z bezpieczeństwa IT oraz dostęp także do materiałów z edycji Sekurak Academy z roku 2023!

Przy zakupie możecie skorzystać z kodu: pasja-akademia - użyjcie go w koszyku, a uzyskacie rabat -30% na bilety w wersji "Standard"! Więcej informacji na temat akademii 2024 znajdziecie tutaj. Dziękujemy ekipie Sekuraka za taką fajną zniżkę dla wszystkich Pasjonatów!

Akademia Sekuraka

Niedawno wystartował dodruk tej świetnej, rozchwytywanej książki (około 940 stron). Mamy dla Was kod: pasja (wpiszcie go w koszyku), dzięki któremu otrzymujemy 10% zniżki - dziękujemy zaprzyjaźnionej ekipie Sekuraka za taki bonus dla Pasjonatów! Książka to pierwszy tom z serii o ITsec, który łagodnie wprowadzi w świat bezpieczeństwa IT każdą osobę - warto, polecamy!

...