Skip to content
This repository has been archived by the owner on Aug 5, 2022. It is now read-only.

Tests #12

Merged
merged 49 commits into from
Apr 26, 2019
Merged

Tests #12

merged 49 commits into from
Apr 26, 2019

Conversation

developer239
Copy link
Contributor

@developer239 developer239 commented Apr 21, 2019

Because I had a pretty long bike ride yesterday, I decided to write some tests. My PR works fine with week 5 homework. It is going to be interesting to see what happens when we add some redux middleware.

However, I am pretty confident that my tests will not break because I am not testing implementation.

On the backend, I am usually writing tests for controllers (routes) Here I also decided to test individual routes. (pages)

A possible disadvantage is that one test can fail for many different reasons. However, I am kind of OK with that because I want my tests to fail if there is a bug somewhere.

With that said I am definitely not an expert so I would appreciate a review. 🙂

TODO:

  • Add cypress example

developer239 and others added 30 commits April 21, 2019 12:05
- we are testing action in our reducer integration test
- just to show how coverage works and why button was not 100 % covered
- create render with router helper
- create basic test case
- add fetch-mock dev dependency
- mock GET /api/skus
- mock POST /oauth/token
- initialize redux store with mock data
- update cart item component so that we can differentiate between loaded and not loaded state
- test unsuccessful submit
- test successful submit
…hts-2018-private into tests

# Conflicts:
#	src/components/Layout/index.js
#	src/pages/Cart/CartItem.js
#	src/pages/LogIn/index.js
#	src/pages/SignUp/index.js
Copy link
Contributor

@dannytce dannytce left a comment

Choose a reason for hiding this comment

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

Wow, usually after a long ride on a bike I am not writing 2k lines of code 😅 Please consider if it's not too much (especially thinking of missing cypress example). In the end, you need to present all the stuff for 1 hour. That might be a bit overwhelming both for you and students.

Anyway thank you very much for contribution! Now let's take a look more deeper

  1. Regarding [DO NOT MERGE YET]. There is a Draft Pull Request for these purposes. Or personally, I prefer to put [WIP] prefix in front of the name of PR.

  2. Let's get rid of index.test.js and name it properly according what this test is going. E.g. Button/tests/index.test.js will become a Button/tests/Button.test.js

  3. There is too much snapshot testing. I would rather try to avoid it and bring in actual testing.

  4. I left there a few comments, please take a look on them.

package.json Show resolved Hide resolved
src/App.js Outdated Show resolved Hide resolved
src/pages/Account/test/index.test.js Outdated Show resolved Hide resolved
src/pages/Cart/test/index.test.js Outdated Show resolved Hide resolved
src/pages/ProductList/test/index.test.js Outdated Show resolved Hide resolved
src/store/cart/reducer.test.js Outdated Show resolved Hide resolved
@developer239 developer239 changed the title Tests [DO NOT MERGE YET] [WIP] Tests Apr 22, 2019
- fix naming in reducer test
- rename test files from index.test.js to [testee].test.js
- replace some snapshots with normal jest expectations
…-private into tests

# Conflicts:
#	src/components/Layout/index.js
#	src/components/Pagination/SizeSelect.js
#	src/pages/Cart/CartItem.js
#	src/pages/Cart/index.js
#	src/pages/LogIn/index.js
#	src/pages/ProductList/index.js
#	src/pages/SignUp/index.js
#	src/store/cart/reducer.js
#	yarn.lock
@developer239 developer239 changed the title [WIP] Tests Tests Apr 24, 2019
@dannytce dannytce merged commit 9502fdc into master Apr 26, 2019
@dannytce dannytce deleted the tests branch April 26, 2019 08:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants