Czy jest coś złego w moim kodzie? Testowanie aplikacji

1

Może moje pytanie wyda się newbie, ale nadal tak się czuję. Pokaże na prymitywnym przykładzie:

Mam w środku aplikacji javowej gdzieś w logice biznesowej nazwijmy to QuestionService metodę:

    @Transactional
    public boolean checkUserAnswer(int questionId, String userAnswer) {
        Question q = findById(questionId);
        Exam exam = examService.findByQuestionId(questionId);

        Predicate<Answer> correctAnswer = a -> advancedComparator.isEqual(userAnswer, a.str(), exam.settings());
        boolean isUserAnswerCorrect = q.getAnswers().stream().anyMatch(correctAnswer);

        if(isUserAnswerCorrect) {
            q.markGoodAnswer();
        } else {
            q.markWrongAnswer();
        }

        return isUserAnswerCorrect;
    }

operuje tam sobie na modelu wyciągam encje ExamSettings i przekazuje do AdvancedComparator metody
public boolean isEqual(String userAnswer, String answer, ExamSettings exam) - ta klaska korzysta sobie z modelu ExamSettings, z kilku booleanów READONLY.

Wysoko od strony serwisów mam też ExamSettingsDTO i metode w ExamService typu:

    @Transactional
    public void changeExamSettings(ExamSettingsDTO settings) {
        ExamSettings newSettings = ef.createExamSettingsFromDTO(settings);
        examSettingsRepository.save(newSettings);
    }

ef to EntityFactory/konwerter która tworzy poprawne obiekty modelu z dto i ma metodę:

public ExamSettings createExamSettingsFromDTO(ExamSettingsDTO dto) {
        Exam exam = examService.findById(dto.getExamId());

        ExamSettings settings = new ExamSettings();
        settings.setExam(exam);
        settings.setExamId(dto.getExamId());
        settings.setIgnoreBlank(dto.isIgnoreBlank());
        settings.setIgnoreCase(dto.isIgnoreCase());
        settings.setIgnoreDiacritics(dto.isIgnoreDiacritics());
        settings.setIgnoreFirstAndLastSpaces(false); //TODO
        settings.setIgnorePunctuation(dto.isIgnorePunctuation());
        settings.setIgnoreSpaces(dto.isIgnoreSpaces());

        return settings;
    }

dodatkowo setery modelu ExamSettings są package bo kodzie jest taka konwencja,
że spoza niektórych pakietów nie pozwalam od tak sobie tworzyć obiektów modelu przez konstruktor. do tego trzeba używać/pisać konwerter-fabryke!

Teraz jednak chciałabym przetestować klasę AdvancedComparator metodę advancedComparator.isEqual(userAnswer, correctAnswer, examSettings).
Najłatwiej byłoby mi stworzyć w teście new ExamSettings(), seterami ustawić tych kilka booleanów z których ta metoda isEqual korzysta i ją wywołać.
Zwłascza jak z ExamSettings ta metoda olewa np. exam, examId itd... Jednak przez package settery i tą konwencję całą tak nie mogę tak w teście zrobić.
Muszę robić inaczej -niżej przykładowy kod testu i komentarze ilustrujące moje podejście:

@Test
    public void ignoreSpaceShouldIgnoreNonBreakingSpaceToo() {
        // given
		// tworze egzamin. olewam, stworzenie do egzaminu pytania, bo metoda advancedComparator.isEqual ma to gdzieś
        final int examId = examService.createExam("non breaking space anty hack");

        final String correctAnswer = "<- non breaking space here";
        final String userAnswer = "       <- non breakingspace here";

		// tworze ExamSettingsDTO. olewam, wywołania typu examService.changeSettings , bo przetestuje sobie bezpośrednio
		// advancedComparator.isEqual i model dostarcze w teście.
        ExamSettingsDTO examSettings = new ExamSettingsDTO();
        examSettings.setExamId(examId); // ustawiam id żeby fabryka sobie poradziła z konwersją
        examSettings.setIgnoreSpaces(true);

		// używam fabryko-konwertera żeby dostać encje modelu. (w bazie tak naprawdę egzamin ma inne settings model)
        ExamSettings examSettingsModel = entityFactory.createExamSettingsFromDTO(examSettings);

        // when

		// wywołuje bezpośrednio gdzieś wybebeszoną metodę advancedComparator.isEqual z tym co chce.
        boolean isCorrect = advancedComparator.isEqual(userAnswer, correctAnswer, examSettingsModel);

        // then
        assertTrue(isCorrect);
    }

Obiło mi się o uszy, że jak coś jest trudne w testowaniu to jest źle napisane.

  1. Czy jest coś co byście tutaj zmienili / poprawili albo są błędy w moim myśleniu?
  2. Może najlepiej środek aplikacji też testować wywołując metody z góry/z serwisów? czyli przykładowo u mnie mogę tego AdvancedComparatora sobie przetestować za pomocą checkUserAnswer (na samej górze jest kod tej metody). Aczkolwiek nie wiem jak to się ma do testowania konkretnie jednej rzeczy i prostoty. Przy testowaniu od góry,
    będę musiała użyć z 3 serwisów i normalnie stworzyć egzamin, pytania, zmienić ustawienia egzaminu, wywołać checkUserAnswer a tak naprawdę chce przetestować
    tylko jedną prostą metodę z środka.
  3. Czy konwencja fabryk/konwerterów jest okey?
  4. advancedComparator.isEqual czyta model read only - może ta metoda powinna przyjmować inny obiekt, który w teście będę mogła stworzyć przez new a ExamSettings będę po prostu do tego konwertować w środku logiki?
  5. Może wszystko jest robię dobrze i nie ma się czym przejmować?

Wiem jak wygląda kod w większych komercyjnych projektach i że to są problemy pierwszego świata, ale chcę zweryfikować poprawność swoje podejścia i myślenia na prymitywnym przykładzie.

PS więcej kodu mogę dostarczyć
PS2 najłatwiej mi rozumieć na przykładach

1

Hej jeśli test jest w tym samym package co Klasa, która testujesz to powinna mieć dostęp do pól/metod, które mają ustawiony defualtowy dostęp.
Jeśli chodzi o te konwertery, to dlaczego akurat tak? Ja używam na poziomie dao i serwisu obiektów Modelowych, które ewentualnie konwertuje do postaci obiektów Data, które np. wyświetlam, lub wysyłam do innego systemu zewnętrznego.

0

@Black007 test nie jest w tym samym pakiecie co klasy z setterami defaultowymi.

Jeśli chodzi o te konwertery, to dlaczego akurat tak? Ja używam na poziomie dao i serwisu obiektów Modelowych, które ewentualnie konwertuje do postaci obiektów Data, które np. wyświetlam, lub wysyłam do innego systemu zewnętrznego.
Ale jak? no ja też właśnie tak robię przecież

1

"dodatkowo setery modelu ExamSettings są package bo kodzie jest taka konwencja,
e spoza niektórych pakietów nie pozwalam od tak sobie tworzyć obiektów modelu przez konstruktor. do tego trzeba używać/pisać konwerter-fabryke!"

Po co? czyli w pakiecie com.foo tylko obiekty z tego pakietu mogą cokolwiek robić z innymi klasami z tego pakietu, a klasy z pakietu com.foo.bar już nie ?

1

A dobra, sorry skumałem o co chodzi :) - trudno się połapać, jak nie widzi się całości.

  1. taka mała uwaga: nie exam.settings() tylko exam.getSettings() powinno być - jesli to getter?
  2. Są testy jednostkowe i integracyjne. Jeśli chcesz przetestować advancedComparator.isEqual to wrzuć tam spreparowane dane (mocki, o tym dalej) i obiekty i to będzie test jednostkowy. test integracyjny będzie wtedy, gdy przetesujesz jak twój serwis działa razem z tym co nazywasz "środkiem" aplikacji.
  3. Jest ok, tylko fabryki powinny mieć nazwy jak np. SettingFactory.getSettings czy coś w ten deseń.
  4. Możesz też używac czegoś takiego jak Mock - klasa, która implementuje dany interfejs, a nie jest np. pobierana z bazy tylko jest to twoja klasa, która "udaje" klasę modelu
    Polecam to: http://www.vogella.com/tutorials/JUnit/article.html i to: http://www.vogella.com/tutorials/Mockito/article.html
4

To co napisałaś to NIE JEST test jednostkowy. Not even close. Test jednostkowy dla tego kodu wyglądałby np. tak:

@Test
    public void ignoreSpaceShouldIgnoreNonBreakingSpaceToo() {
        final String correctAnswer = "<- non breaking space here";
        final String userAnswer = "       <- non breakingspace here";
        ExamSettings examSettingsModel = createNiceMock(ExamSettings.class); 
        expect(examSettingsModel.isIgnoreWhitespace()).andReturn(true).anyTimes();
        replay(examSettingsModel);

        boolean isCorrect = advancedComparator.isEqual(userAnswer, correctAnswer, examSettingsModel);
 
        verify(examSettingsModel);
        assertTrue(isCorrect);
    }

Popatrz czym sie różni mój kod od twojego. Mój zależy TYLKO od interfejsów a JEDYNA prawdziwa implementacja która jest używana w całym teście to właśnie metoda isEqual.
A takie testowanie przez wołanie metody "u góry" to jest test integracyjny. Też fajny, ale z jednostkowym niewiele ma wspólnego.

0
Black007 napisał(a):

A dobra, sorry skumałem o co chodzi :) - trudno się połapać, jak nie widzi się całości.

  1. taka mała uwaga: nie exam.settings() tylko exam.getSettings() powinno być - jesli to getter?
  2. Są testy jednostkowe i integracyjne. Jeśli chcesz przetestować advancedComparator.isEqual to wrzuć tam spreparowane dane (mocki, o tym dalej) i obiekty i to będzie test jednostkowy. test integracyjny będzie wtedy, gdy przetesujesz jak twój serwis działa razem z tym co nazywasz "środkiem" aplikacji.
  3. Jest ok, tylko fabryki powinny mieć nazwy jak np. SettingFactory.getSettings czy coś w ten deseń.
  4. Możesz też używac czegoś takiego jak Mock - klasa, która implementuje dany interfejs, a nie jest np. pobierana z bazy tylko jest to twoja klasa, która "udaje" klasę modelu
    Polecam to: http://www.vogella.com/tutorials/JUnit/article.html i to: http://www.vogella.com/tutorials/Mockito/article.html
  1. Rzeczywiście
  2. Dzięki za wytłumaczenie teraz to rozumiem. Czyli u mnie te testy serwisów to integracyjne a jednej części takiej jednostkowe. Dobra.
  3. Okey.
  4. Okey wiem co to mock. Zastanawiam się jeszcze jedynie nad Mockito, EasyMock a PowerMock. Generalnie do testowania używam TestNG
1

Ja uzywam EasyMock + Powermock w sytuacjach szczególnych (tzn mockowanie static, mockowanie new itd). Mockito generalnie ma takie same możliwości jak EasyMock więc to kwestia co ci sie bardziej podoba. Mnie bardzo podoba się EasyMock ;)

1

na koniec wrzucę kod jak teraz wyglądają moje testy

public class AdvancedComparatorTest extends AbstractTest {

    @Inject private AdvancedComparator advancedComparator;

    @Test
    public void ignoreSpaceShouldIgnoreNonBreakingSpaceToo() {
        // https://en.wikipedia.org/wiki/Non-breaking_space

        final char NON_BREAKING_SPACE = 0x00A0;
        final String correctAnswer = "<- non breaking space here";
        final String userAnswer = StringUtils.repeat(NON_BREAKING_SPACE,20)
                + "<- non breakingspace here";

        ExamSettings settingsMock = mock(ExamSettings.class);
        when(settingsMock.isIgnoreSpaces()).thenReturn(true);

        boolean isCorrect = advancedComparator.isEqual(userAnswer, correctAnswer, settingsMock);

        verify(settingsMock,atLeastOnce()).isIgnoreSpaces();
        assertTrue(isCorrect);
    }

    @Test
    public void ignoreDiacriticsMarksTest() {
        // https://github.com/apache/commons-lang/pull/105/files

        final String correctAnswer = "ąĄćĆęĘłŁńŃóÓśŚźŹżŻ i zwykly tekst";
        final String userAnswer = "aAcCeElLnNoOsSzZzZ i zwykly tekst";

        ExamSettings settingsMock = mock(ExamSettings.class);
        when(settingsMock.isIgnoreDiacritics()).thenReturn(true);

        boolean isCorrect = advancedComparator.isEqual(userAnswer,correctAnswer, settingsMock);

        verify(settingsMock,atLeastOnce()).isIgnoreDiacritics();
        assertTrue(isCorrect);
    }
}

jest o niebo lepiej i wszystkie moje problemy zniknęły hihi :P
Dzięki

1

Polecam używanie Spocka zamiast junita/testng/mockito które są w tej chwili out of date jeśli porównać je do Spock'a.

Przykładowy link z sieci:
https://dzone.com/articles/introduction-spock

0

@NoZi w czym są out of date? nie chce mi się przepisywać wszystkich testów do Spock'a i to jest chyba groovy , którego nie umiem to dość duża zmiana.

3

@karolinaa
Są out of date bo Spock po prostu bije je każdym aspektem jak łapanie wyjątków, testy parametryzowane, bdd like, pisze się je dużo łatwiej, są czytelniejsze, stubowanie/mockowanie jest wbudowane, masz ładnie wydzielone bloki given/when/then/and i możesz przy nich pisać text który jest również dokumentacją. Masz zajebiste testy parametryzowane których nic nie przebije na rynku bo te z junit czy testng to kpina przy spocku. Grooviego praktycznie nie musisz znać żeby cokolwiek napisać.

Np.

private def settingsMock = Mock(ExamSettings )

def "ignore diactric marks"(){
  given: "strings with diactric marks"
        final String correctAnswer = "ąĄćĆęĘłŁńŃóÓśŚźŹżŻ i zwykly tekst"
        final String userAnswer = "aAcCeElLnNoOsSzZzZ i zwykly tekst"

        settingsMock.isIgnoreDiactrics() >> true 

  when: "comparing given string with diactric marks"
       boolean isCorrect = advancedComparator.isEqual(userAnswer,correctAnswer, settingsMock);

  then: "there was no exception"
       noExceptionThrown()

  then: "the string must be the same"
       isCorrect

  and: "settings collabolator was invoked at least once"
       1 * settingsMock.isIgnoreDiactrics()
}

1 użytkowników online, w tym zalogowanych: 0, gości: 1