search

thybag / RPGCampaignManager

An Opensource RPG Campaign Manager and World Building tool.
3 stars 7 forks source link

test: update models test coverages #11

Closed hatth-1632 closed 3 years ago

hatth-1632 commented 3 years ago

update coverage

image

Refer to issue #5 Pull Request

I dont know how to add label hacktoberfest in pull request. Can you help me?

Thank you

thybag commented 3 years ago

Hi hatth-1632,

Thanks for taking the time to have a look, but I can't really accept this change in its current form.

Reading the changes, this appears to only be adding what is effectively a assertEquals($model->fillable, $model->fillable) to the two models, which fails to provide any additional assurance that the codebase functionality is working as it should. My fear is having tests like this ends up adding test fragility (I'd have to update fillable twice to add an attribute) without providing much insight in to whether the change was correct or has caused an unexpected consequence on another part of the codebase.

If your looking for things that maybe worth expanding test wise, I suspect the update methods in Image may be worthwhile (ie. do the image & preview both get generated and in the correct place. Does uploading the same image debupe etc) - along with controller tests to ensure the policy methods all function correctly.

I've added a bit more detail over on https://github.com/thybag/RPGCampaignManager/issues/5#issuecomment-706519586 if your interested.

thybag commented 3 years ago

Closing as no updates, give me a prod if you'd like me to reopen