simple tests for users and auth

[sanghunjo921]

issue #21

• unit tests for CRUD operations for the user model
• a single test for a user serializer
• figured out how to transform json-formatted data when sending a POST request
• unit tests for registration covering both success and failure cases
• unit tests for login covering both success and failure cases

cover 4 endpoints

1. users/
2. users/pk
3. authentication/registration/
4. authentication/login/

[sanghunjo921]

일단 user model에 한해서 get, get by id, put, delete, post unit tests & serializer 정상적으로 동작하는지 테스트 만들었습니다. 다른 모델도 비슷하게 구현될것 같은데 일단 피드백 받아서 수정해보도록 하겠습니다.

[sanghunjo921]

해볼만한 개선사항 @pytest.fixture def mymodel_data(db): MyModel.objects.create(name="Foo", description="Bar") 와 같이 테스트 디비와 샘플 데이터를 만들어서 각 유닛 테스트마다 반복적으로 create_user() OR register_user() 콜 하지 않게 개선하는것도 괜찮을것 같습니다.

[sanghunjo921]

제가 한건 다 unit test라 모든 crud operations을 하나의 def내에서 처리하는 테스트 하나 추가했습니다. 이렇게 하는게 맞는지 컨펌 부탁드릴게요.

[f-lab-stephen]

오 일단 다 보지는 않았지만 (원래 테스트 케이스는 깔끔하게 짜기 위해서 짜는 코드가 아니라 실제로도 코드리뷰할 때 다 읽지는 않는 편입니다) 방식 자체는 잘 이해하신 것으로 보이네요!

[sanghunjo921]

리뷰 사항

• 개인적으로는 fixture들이 실제 테스트 케이스와 다른 파일로 분리되어 있는 구조를 선호합니다. 이 부분에 대해 어떻게 생각하시나요? 만약 분리한다면 fixture의 파일이 auth, user 두 개가 되는 게 좋을까요 fixtures.py 하나로 몰아 넣는 게 좋을까요?
• 테스트케이스에 localhost URL을 명시적으로 넣는 것도 좋아하지 않습니다. 다른 방법이 있을 수 있을 거라 생각합니다. 다만 localhost에 직접 호출하시더라도 가능하면 URL을 상수화하는 게 좋겠습니다. 나중에 변경할 때 유리합니다.

TEST_URL = "localhost:8000"

생각해보지는 않았었는데 만약 fixure와 테스트 케이스를 분리하고자 한다면 모듈별로 구분하는게 나을거 같네요. 분리를 해야 각 모듈/애플리케이션에 대한 테스트를 작성할거나 fixure를 변경시 필요한 fixure를 손쉽게 찾을 수 있을거 같습니다. 그리고 빠르게 찾아보니까 하나의 파일에 너무 많은 fixure가 존재한다면 각 테스트를 실행시 pytest가 전체를 load하게 되어 performance 측면에서도 나쁠거 같네요.

즉, 분리하게 되면 좀더 organizing되고 재사용성이나 유지보수 및 테스트 퍼포먼스가 유리할거 같습니다.

url 부분은 시간나면 수정해보도록 하겠습니다.

[sanghunjo921]

여러 도큐먼트 참조할때 url 부분을 직접 리퀘스트 날리는 url을 넣으면 된다고 해서 localhost:8000 포함해서 넣었는데 사실 pytest가 제 로컬서버가 돌아가면서 실행되는거라 굳이 명시하지 않아도 정상적으로 작동하네요 ... /admin/ 부분은 그런식으로 처리하고 다른 부분에는 넣어버렸네요.

[sanghunjo921]

test.yaml에 pytest 돌아가게 추가 후 push 했는데 정상적으로 돌아가는거 같네요

[sanghunjo921]

pr쪽은 로컬 main branch에서 pull 이후 test branch에 rebase하고 lock & toml 파일 다른부분 처리하고나서 force로 push 했습니다.

[sanghunjo921]

property 관련 테스트 작성하고 있습니다.

update시 모든 필드를 다 payload로 보내야 하는게 이상해서 찾아보니 patch란것도 있었군요. view 다시 작성후 포스트맨 및 pytest 정상적으로 동작하게 변경했습니다.

[f-lab-stephen]

어휴 훌륭하네요. 커버하고자 하는 부분 다 커버 되면 말씀해 주세요!

[sanghunjo921]

property & admin test 작성하면서 문제점이 있는듯 합니다.

두 모델 모두 user_id를 foreign key로 가지고 있는데 현재 url상으로는 user_id를 가져올 방법이 없는거 같아요. 이를 해결하기 위해 users/{user_id}/properties, users/{user_id}/agents로 변경하고자 하는데 괜찮을까요?

[f-lab-stephen]

네, 그렇게 수정하셔야 할 것 같네요!