search

million-views / praas

Proxy as a service.
MIT License
4 stars 2 forks source link

crud-server, test: increase code coverage #139

Closed shinenelson closed 4 years ago

shinenelson commented 4 years ago

I noticed some easy-to-reach code that were not being covered in the test suite.

in the process, I also found a bug with the PUT method for the user ( update user information ) which would fail because a model method was ~not implemented. I could not find any references to it in the history~ ~either ( it could also be that I didn't look hard enough ). I implemented~ ~the function and wrote the test for the method.~ missing. this was because Sequelize renamed the findById method to findByPk in v5. this was missed during the upgrade from v4 to v5 in a14cd84.

also while implementing the test for the PUT method for the user, I wondered what would happen if I attempted to change the email of the user since we use the user's email as the primary key. while the request processed successfully, the email address field was not updated which is good. shouldn't we throw some kind of error in those scenarios though? I'm not sure what should be done in such scenarios, hence I didn't include that in the update data for the user. I'm currently only changing the name of the user. if an error condition is supposed to be triggered ( and if it is not implemented ), I can add the code for it.

code coverage report

$ npm run test-rest-with-coverage

before

File % Stmts % Branch % Funcs % Lines
All files 70.42 56.76 75 70.73
src 51.56 25 16.67 51.56
passport.js 69.23 50 100 69.23
src/models 90.28 75 96.15 89.39
conduit.js 86.84 75 100 84.38
user.js 92.59 75 87.5 92.59
src/routes/api 62.91 48 64.71 63.97
conduits.js 60.67 47.06 85.71 62.03
users.js 66 50 50 67.39

after

File % Stmts % Branch % Funcs % Lines
All files 73.67 62.16 80.7 74.24
src 53.13 33.33 16.67 53.13
passport.js 76.92 100 100 76.92
src/models 93.24 83.33 96.3 92.65
conduit.js 89.47 80 100 87.5
user.js 96.55 100 88.89 96.55
src/routes/api 68.21 54 82.35 69.85
conduits.js 61.8 50 85.71 63.29
users.js 80 66.67 87.5 82.61

unchanged

File % Stmts % Branch % Funcs % Lines
src
server.js 47.06 20 0 47.06
src/config 100 75 100 100
index.js 100 75 100 100
src/lib 80 66.67 66.67 81.63
helpers.js 80 66.67 66.67 81.63
src/models
index.js 100 100 100 100
system.js 100 100 100 100
src/routes 100 83.33 100 100
auth.js 100 83.33 100 100
index.js 100 100 100 100
src/routes/api
index.js 66.67 50 50 63.64
shinenelson commented 4 years ago

for the record, we have decided that we will not spend time on triggering an error when an email is passed to PUT /user, for now.