Skip to content
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

Encja cart testy #21

Merged
merged 4 commits into from
Feb 19, 2019
Merged

Encja cart testy #21

merged 4 commits into from
Feb 19, 2019

Conversation

przemyslawkrawczynski
Copy link
Collaborator

Poprawiony plik, przepraszam za powszednią gałąź i zamieszanie. Jednak mam pewną zagwostkę w tescie getCart specjalnie zakomentowałem jeden test. po odkomentowaniu test się wykonuje, ale projekt się nie buduje z czystej ciekawości ktoś podpowie co może być przyczyną?


//Then

// Assert.assertEquals(expected, cartValue, 0.1);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ten test po odkomentowaniu u mnie się wykonuje, jednak projekt nie chce wstać, zastanawia mnie o co kaman?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

u mnie ten test po odkomentowaniu się nie wykonuje ponieważ cr nie może znaleźć koszyka o podanym przez ciebie id. w tym przypadku nie wiesz jakie id zostało wygenerowane dla testCart, więc zamiast 1L powinienaś pobrać id które zostało przypisane do testCart.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mamy autonumeracje w Encji, i H2 zaczyna nie od 0 tylko od 1. Tabela wyczyszczona więc musi to być 1L (na tym polega test że coś zakładamy) i generalnie to powinno funkcjonować z założeń o których wiem, pytanie czego nie wiem lub o czym nie pomyslałem...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kwestia numeracji. Zrobiłem "bezczelne" System.out.println("Cart id: " + testCart.getId()); którego wynikiem było: "Cart id: 6"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No i właśnie ja też na 3 różnych maszynach i ciągle wychodzi mi wspomniana jedynka. No nic zgodnie z zaleceniem jutro wrzucę poprawione. Tak jak mówiłem bardziej ludzka ciekawość i gdzie leży błąd z czego wynika rozbieżność... Wytyczne jasne ma działać i się kompilować ;) dzięki chłopaki za review

Copy link
Collaborator

@kszubra kszubra Feb 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Głowy nie dam, ale wydaje mi się, że może chodzić o tabelę hibernate_sequence. Kiedy robiłem testy w innym projekcie, to mimo iż obiekty były usuwane z bazy, to ich id były wciąż "zajęte" i nowe obiekty dostawały kolejne id. Dopiero drop tabeli hibernate_sequence powodował "wyzerowanie" i przydzielanie id od "1". Ale było to w przypadku bazy SQL, nie H2. Jeśli kombinuję w złą stronę to proszę poprawcie mnie :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

w tej kwestii to ja bym nie był taki krytyczny względem @przemyslawkrawczynski . Natomiast @przemyslawkrawczynski też powinieneś wziąć pod uwagę, że u kogoś może coś nie przechodzić. Tutaj to jest klasyczna standardowa odpowiedź administratora numer 1 (SOA1), czyli "u mnie działa". Generalnie fajnie by było gdyby testy wykonywały się zawsze - niezależnie od stanu bazy. Więc albo sprawdzamy ID, który został zwrócony z bazy danych albo robimy pełny tearUp() i tearDown(). Ogólnie unit testy robicie na H2 ale może będziecie chcieli kiedyś odpalić pełną integrację na rzeczywistej bazie jak Oracle czy MySQL? Implementacje testów macie już gotową i gdyby testy nie były zależne od stanu bazy to wtedy możemy podmienić tylko "sterownik" na MySQL i zrobić pełne testy integracyjne z produkcyjną bazą danych... wtedy możemy np. dodać do pipeline w CI/CD albo ustawić, że ma lecieć raz dziennie w ciągu nocy na środowisku przed-produkcyjnym.

Mam nadzieję, że trochę światła rzuciłem w tym temacie :-)


//Then

// Assert.assertEquals(expected, cartValue, 0.1);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

u mnie ten test po odkomentowaniu się nie wykonuje ponieważ cr nie może znaleźć koszyka o podanym przez ciebie id. w tym przypadku nie wiesz jakie id zostało wygenerowane dla testCart, więc zamiast 1L powinienaś pobrać id które zostało przypisane do testCart.

testCart.getProducts().add(new Product("p1", "Description p1", new BigDecimal(55.55)));
cr.save(testCart);

int cartProductListSizeAfterAdding = cr.findById(1L).orElseThrow(CartNotFoundException::new).getProducts().size();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jak wyżej wspomniałem id powinienaś pobierać z koszyka.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Zgadzam się, bezpieczniejsza opcja kiedy nie ma się pewności jak wygląda "licznik" id.

testCart.getProducts().add(p3);

cr.save(testCart);
int cartListSizeAfterAdding = cr.findAll().size();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uważam że najpierw powinieneś pobrać ilość wpisów w bazie ponieważ jakieś wpisy mogą się już tam znajdować a wtedy asserty nie będą zgodne.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

W przypadku testu na bazie H2 będą zgodnę, bo przed każdym postawieniem testu Hibernate kasuje jeśli by istniały te tabelki i tworzy na nowo:

Hibernate: drop table carts if exists
Hibernate: drop table generic_entity if exists
Hibernate: drop table join_cart_product if exists
Hibernate: drop table orders if exists
Hibernate: drop table products if exists
Hibernate: drop table products_groups if exists
Hibernate: drop table users if exists
Hibernate: drop sequence if exists hibernate_sequence

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uważam że testy powinny się wykonywać poprawnie niezależnie od tego jakiej bazy będziemy używać w testach.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@radion358 częściowo ma rację. Jednak jeśli pójdziemy tą drogą to trzeba będzie pisać cały tearUp() i tearDown() gdzie dodamy rekordy a następnie je pod koniec usuniemy.

spring.datasource.password=sa
spring.jpa.show-sql=true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fajne. Jak znasz coś co zamieni znaki zapytania na wartości to też dodaj :)


//Then
Assert.assertEquals(expectedResult, quantity);
Assert.assertEquals(expectedResult2, productQuantityInDatabase);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No to te testy nie są zgodne z tym co ostatnio sprawdzałem. Co prawda testowałem to i w obecnej konfiguracji H2 zwija transakcje po przeprowadzeniu testów, jednakże jeśli coś będzie w bazie danych to test ten może nie przejść, bo np. zwróci o jeden wynik więcej.

Zerknij tutaj:
https://github.com/pplutap/JDP-1901-01/pull/18/files#r256634167

Niby H2 startuje od 0 i tworzy strukturę itp... ale chłopaki poszli chyba w inną stronę zanim jeszcze ja zrobiłem review i dostałem info, iż tak ustaliliście. Co teraz?

cr.save(testCart);
List<Cart> cartList = cr.findAll();

// Double cartValue = cr.findById(1L).orElseThrow(CartNotFoundException::new)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

te komentarze to wylecą ?

testCart.getProducts().add(p3);

cr.save(testCart);
int cartListSizeAfterAdding = cr.findAll().size();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@radion358 częściowo ma rację. Jednak jeśli pójdziemy tą drogą to trzeba będzie pisać cały tearUp() i tearDown() gdzie dodamy rekordy a następnie je pod koniec usuniemy.


@Transactional
@Test
public void testFindAll() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test nazywa się "findAll" a testuje dodawanie zamówień do orderService poprzez getOrderList... dopracujemy ? :)

List<Order> orders = orderService.getOrderList();

//Then
assertTrue(orders.contains(order));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to jest tożsame z testem testFindAll(). Testujesz dokładnie to samo tylko w inny sposób. Tam dodajesz 3 zamówienia poprzez addOrder() i sprawdzasz czy rozmiar się zmienił. Tutaj dodajesz 1 zamówienie i sprawdzasz czy pojawiło się na liście.

@przemyslawkrawczynski przemyslawkrawczynski merged commit f4c84a8 into master Feb 19, 2019
@przemyslawkrawczynski przemyslawkrawczynski deleted the Encja_Cart_Testy branch February 19, 2019 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants