passport spec #77
passport spec #77
Conversation
| findAndUpdatedModel.workspaceName = | ||
| fakeData.updateNewData.workspaceName; | ||
| findAndUpdatedModel.passwordHash = | ||
| fakeData.updateNewData.passwordHash; |
There was a problem hiding this comment.
這是測試建立 data 的麻？
為什麼這裡還會有修改 data 的行為？
L96 - L103 我應該會拉出去寫一個function
EX:
我還是會比較習慣用 update 去做
const updateData = (modal, {updateNewData}) => {
const newModal = {...lmodal, ...updateNewData};
return newModal.save();
}
There was a problem hiding this comment.
QQ 這裡是我建好的 init data 缺了一個 notNull 欄位的資料，所以我請 @chenpion 直接補上
There was a problem hiding this comment.
了解！
不過 initial data 不是應該會在 beforeAll or beforeEach ?
There was a problem hiding this comment.
正常是不會，但在測試 update() / destroy() 時會需要在 before 中建立，因為必須要先有資料才能做以上兩個操作
以這裡來說， @chenpion 這裡的變動是寫在 spec 最前端，所以應該比較沒有疑義？
參考：
| }, | ||
| }); | ||
|
|
||
| if (findAndUpdatedModel) { |
There was a problem hiding this comment.
這邊就不用包 if 了，因為這是 unit test，所以 findAndUpdatedModel 一定不能是 null
包 if 反而如果有問題不能發現
| expect(findAndUpdatedModel[e]).to.equal(fakeData.updateNewData[e]); | ||
| }); | ||
| try { | ||
| let findAndUpdatedModel = await models[modelName].findOne({ |
There was a problem hiding this comment.
前面是用 findOne
這邊卻用 findAndUpdateModel
唸起來怪怪的
他應該只是一個 userdata?
他回傳的應該還是個物件
只是有一個 wrapper 提供你直接修改 DB的街口
但是他本質還是 data
There was a problem hiding this comment.
這裡的變數是我命名的QQ
因為這個單元測試有指定操作方法，這裏想表達的含義是該 model data 是需要使用 find 去做 update，所以我用這個 findAndUpdatedModel 表示該 model 是 find 之後 updated
不過命名的確有改空間
| }, | ||
| }); | ||
|
|
||
| let updateModel = await models[modelName].findOne({ |
There was a problem hiding this comment.
這一個 modelName 是一個常數
應該可以改成 PASSPORT_MODEL = 'Passport';
這樣後面再讀的時候才不用都拉到最上面去找這個 modelName 是什麼
There was a problem hiding this comment.
這裏跟 user.spec 是一樣的原因，modelName 作用是讓大部分 spec 可以共用的 model 名稱參數，不過因為用途是常數，應該改為大寫沒錯
建議 @chenpion 改為 SPEC_MODEL_NAME
There was a problem hiding this comment.
了解
因為每個檔案都會定義自己的 SPEC_MODEL_NAME
共用的意義似乎不是太大 ＱＱ
如果變數可以直接體現出這個 Model Name
review 上應該會比較輕鬆
不過這只是 SPEC 檔案就有分開了
應該也還好 ＸＤ
There was a problem hiding this comment.
因為我等於是寫了第一份 spec 之後，改這個 modelName 再複製出來，就可以讓單元測試的大部分程式碼共用於全部的 model，我再針對某些 model 彼此特殊的關係開細部測試規格給大家即可
而你說的直接從變數名稱上看到是哪個 model 這件事，因為以 passport.spec.js 這個檔案來說，它就是針對路徑 models/passport 這個 model 來做單元測試，我想應該識別度上沒有問題才是？
There was a problem hiding this comment.
辨識度上應該還好
只是如果能夠更符合變數的命名
對review 或是之後承接的人會比較輕鬆一點
| @@ -139,11 +184,15 @@ describe(`models/${modelName}`, () => { | |||
| // 1. use model.findAll... | |||
| // 2. use data from `fakeData.findAll` | |||
| // 3. use `findAllModel` as target, and use `fakeData.keyword` as option. | |||
| let findAllModel = null; | |||
| let findAllModel = await models[modelName].findAll({ | |||
There was a problem hiding this comment.
同上
findModel 似乎應該是 function (尋找model)
他這邊卻是變數
| // 1. model.create(fakeData.create)... | ||
| // 2. use data from `fakeData.create` | ||
| let createdModel = null; | ||
| let createdModel = await models[modelName].create(fakeData.create); |
There was a problem hiding this comment.
Tomas 建議如果沒有重新賦值會使用 const 來宣告
No description provided.