Skip to content

[3주차] 신혜리#4

Open
rachel490 wants to merge 36 commits into
submissionfrom
develop3
Open

[3주차] 신혜리#4
rachel490 wants to merge 36 commits into
submissionfrom
develop3

Conversation

@rachel490

Copy link
Copy Markdown
Owner

3주차 신혜리

접근 방법

  • 테스트 코드가 직전 미션들에서 많이 미흡하다고 생각하여 이번에는 테스트 코드부터 짜는 연습을 하였습니다.
  • 테스트 코드 작성 후 테스트 코드가 통과할 가장 기본적인 코드만 작성하였고 후에 스타일을 완성하고 마지막 리팩토링을 진행하였습니다.
  • 리팩토링후에도 테스트코드는 변함없이 통과가 되어야한다고 했는데 이부분에서 계속적인 undefined 에러가 떠서 결국 마지막까지 해결을 하지 못하였습니다.
  • 저번주와 마찬가지로 dummy data를 만들어서 동적인 데이터를 구현하려고 하였습니다. 하지만 dummy data를 item list 용도로만 제작한 것 같아서 추후 전체적인 쇼핑몰 사이트를 구현할때 통일될 수 있는 dummy data를 만들 필요성이 있다고 느꼈습니다.
  • 미션에서 컴포넌트를 분리하라고 말씀하셔서 Header, Footer을 분리하였고 컴포넌트에 맞춰서 테스트 케이스도 분리하였습니다.

특이사항

  • 자세한 이유를 알수 없으나 리팩토링 후 테스트 케이스가 통과되지 않는 문제점이 발생하였는데 아직 해결을 하지못한 부분이라 이유가 궁금합니다! img가 undefined된다고 에러가 뜹니다..
  • computed의 경우 if문을 쓰려고 했으나 return이 필수적이라고 에러가 떴습니다. 그래서 변수를 하나 만들고 if/else에 따라서 변수의 값을 달리하여 마지막에 해당 변수를 return하는 방식으로 하였는데 왜 return이 필수인가요?
  • 컴포넌트를 분리하고 테스트 케이스도 분리하려고 할때 생각보다 footer, header의 경우 테스트 코드가 많지 않아서 분리할 필요성을 못느꼈는데 컴포넌트별로 파일을 분리했을 때 테스트 케이스도 이에 맞춰 분리하는 것이 필수일까요? 아니라면 테스트 케이스별로 wrapper을 특정 컴포넌트로 해도 상관이 없을 까요?

제출 후 확인

  • 본인의 구현 결과를 dev 브랜치에 구현하고, dev 브랜치에서 submission 브랜치로 PR을 보내주세요.

CC: @externship-master @lkaybob

lkaybob and others added 30 commits January 11, 2022 23:26
* Add template code for week 2
* Add Mission.md for week 2
* Add ESLint checker on PR
* Remove image with commit hash
* Add Mission.md
* Update Github Actions command
* Update pull request template
* Add template code commit hash

@hogyun3709 hogyun3709 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

의견제시 / 의문점

  • 데이터상에서 discountPrice 가 없었다면 computed property로 제품의 할인율이 적용된 값또한 template상에서 보여줄수 있었을것 같습니다.
  • PR에서 props의 undefined를 겪으셨다고하셨는데 혹시 TC 상에서 어떤 케이스에서 겪으셨는지 알수있을까요? 저도 동일한 문제를 겪어서 궁금합니다.

@@ -0,0 +1,42 @@
const products = [

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

mock data를 external file로 구조설계 하셨네요~ array의 각 object 마다 직접적으로 id값을 명명해주는건 어떨까요?

@@ -0,0 +1,42 @@
const products = [

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

mock data를 external file로 구조설계 하셨네요~ array의 각 object 마다 직접적으로 id값을 명명해주는건 어떨까요?

let percent;

if (this.product.onSale) {
const { originalPrice, discountPrice } = this.product;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

아.. 이런식의 코드작성을 통해 반복적인 내용을 줄일수 있었던 내용이 기억나네요 혹시 관련 키워드가 어떤 것인지 여쭤봐도될까요?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

mook data에 onSale을 true, false로 넣어서 그걸 토대로 계산하는군요.
호균님이 말하신 것처럼 반복적인 내용을 줄일 수 있는 코드 작성이 인상깊네요.
생각 못했는데 배워갑니다!

<div id="item-list-page">
<Header />
<ul data-test="product-list">
<item v-for="(item,idx) in productData" v-bind:key="idx" v-bind:product="item" />

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Item component의 작성을 두번해주신 이유가 있을까요? 2열로 표시되어야하는 부분을 만족하기 위한 작성인가요?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

제가 보기에도 2열로 표시되어야 하는 부분을 만족하기 위해서 두 번 해주신 것 같네요. 저는 Item 컴포넌트를 감싸는 div를 하나 만들어서 해결했습니다!

@Subinhyun Subinhyun left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

mook data 파일을 따로 만들어서 구현한 점이 인상깊었습니다.
그리고 코드를 간결하게 작성하려고 노력한 것 같습니다.
저도 하위 컴포넌트의 테스트 코드를 작성할 때 img undefined되어 오류가 났었는데 부모컴포넌트로 바꾸니까 일시적인 오류는 사라졌습니다.. 정확한 이유를 알고 싶네요 !
이 오류에 대해서 디스코드로 다른 분들과 얘기했었는데, 그룹3 한광재님이 wrapper = mount(컴포넌트, {propsData: {}})를 통해 할당하거나 props가 없어도 코드가 되게 validation 처리(ex.옵셔널체이닝)하는 방법 2가지를 알려주셨는데 참고하시면 좋을 것 같네요 ,,
그리고 computed는 template 내부에 선언된 computed 중에서 해당 함수와 연결된 값이 바뀔 때만 해당 함수만을 실행하기 때문에 return하는 것이 아닌가 생각이 드네요 !

let percent;

if (this.product.onSale) {
const { originalPrice, discountPrice } = this.product;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

mook data에 onSale을 true, false로 넣어서 그걸 토대로 계산하는군요.
호균님이 말하신 것처럼 반복적인 내용을 줄일 수 있는 코드 작성이 인상깊네요.
생각 못했는데 배워갑니다!

<div id="item-list-page">
<Header />
<ul data-test="product-list">
<item v-for="(item,idx) in productData" v-bind:key="idx" v-bind:product="item" />

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

제가 보기에도 2열로 표시되어야 하는 부분을 만족하기 위해서 두 번 해주신 것 같네요. 저는 Item 컴포넌트를 감싸는 div를 하나 만들어서 해결했습니다!

const wrapper = mount(Footer);

expect(wrapper.get('[data-test="footer"]').exists()).toBe(true);
expect(wrapper.get('[data-test="footer"]').text()).toBe('홈찜장바구니마이페이지');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

이렇게 한꺼번에,, 테스트 할 수 있겠네요 !

@xialGuri

Copy link
Copy Markdown

computed property를 이용해서 할인율이 적용된 값을 템플릿 상에서 보여줬으면 더 좋았을 것 같습니다!!
데이터를 따로 빼서 만든신점이 좋았습니다. 저도 그 방법을 쓰다가 중간에 오류가 나서 바꿨는데 제대로 구현을 해주셔서 인상깊었습니다! 또한 footer, header를 따로 컴포넌트화 해주신 점도 좋았습니다.

@lkaybob lkaybob left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

기능적인 요구사항을 만족하여 애플리케이션 코드들도 적절하게 작성해주신 것 같지만, 테스트 코드는 통과가 되는 코드를 작성해주신 것인지 의문이 듭니다. 자세한 내용은 inline comment들을 확인해주세요

<img data-test="img" v-bind:src="product.img" />
<div>
<span data-test="discountRate" id="discountRate" v-if="product.onSale">
{{this.discountRate}}%

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

  • this는 필요없습니다.
  • %를 붙이는 부분이 필요하다면 computed property에 함께 포함시켜주세요

const { originalPrice, discountPrice } = this.product;
percent = Math.round(((originalPrice - discountPrice) / originalPrice) * 100);
} else {
percent = null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

  • 질문주신 에러의 경우 editor에서 문제가 되는 에러를 확인하실 수 있습니다. 이 경우는 아래의 rule 때문에 그렇습니다.
  • 기본적으로 data/computed property에 접근할 때 해당 property가 undefined임에도 계산식에 사용되거나 rendering 시도가 될 경우 에러를 내뱉으면서 렌더링이 깨질 수 있습니다. 이러한 rule을 강제하면 모든 경우의 수에 대한 return 구문을 강제헤 렌더링 오류의 가능성을 줄일 수 있습니다.

<Header />
<ul data-test="product-list">
<item v-for="(item,idx) in productData" v-bind:key="idx" v-bind:product="item" />
<item v-for="(item,idx) in productData" v-bind:key="idx" v-bind:product="item" />

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

똑같은 상품이 왼쪽/오른쪽에 표시가 되는 결과를 보이게 됩니다. 상품 목록이 2열로 차례대로 보이게 하는 것은 CSS를 적용해보시면 됩니다.

const wrapper = mount(Footer);

expect(wrapper.get('[data-test="footer"]').exists()).toBe(true);
expect(wrapper.get('[data-test="footer"]').text()).toBe('홈찜장바구니마이페이지');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

테스트하려는 대상을 알기 어렵습니다. 각 메뉴를 찾아서 개별적으로 테스트하시는 것이 더 좋아보입니다.

it('renders product image', async() => {
const wrapper = mount(Item);

await wrapper.setData({

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Item.vue에서는 props로 정의하셨는데 setData로 주는 것은 올바른 데이터 인입 방법이 아닙니다.

  • mount의 props 옵션을 쓰거나 setProps를 사용해야 합니다.
  • 전반적으로 테스트 코드들이 통과가 되는 것인지 의문이 듭니다.

})

expect(wrapper.find('img').exists()).toBe(true);
expect(wrapper.find('img').attributes('src')).toEqual(wrapper.vm.product.img);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

(전체 테스트 케이스에 해당) vm에 직접 접근하여 테스트하는 것은 테스트 코드를 작성하는 의미가 없게 되는 것입니다.

  • 주입한 테스트 데이터를 별도의 변수로 빼서 이를 toEqual의 parameter로 넣어주셔야 합니다.
  • img가 undefined가 되는 것은 컴포넌트에서 product를 prop으로 정의했지만 테스트 코드에서 data로 주고 있어 mismatch로 인해 undefined가 나는 것으로 보입니다.

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