Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update category unit model tests #546

Merged

Conversation

WenInCode
Copy link
Contributor

What's in this PR?

Update category unit model tests

References

@joshsmith
Copy link
Contributor

Do you not want to test the attributes of the Category model here, too?

@WenInCode
Copy link
Contributor Author

@joshsmith just following the ember model testing guide, my understanding is that there is value in testing functions, computer properties and relationship declarations. I'm not sure how much value there is in testing plain attributes... Unless there is validations on them

@begedin
Copy link
Contributor

begedin commented Oct 2, 2016

Same as #547 (comment)

It would make sense to add a test that ensures all attributes are accounted for.

@WenInCode
Copy link
Contributor Author

I've updated this to test attribute existence. Reference this for method reasoning.

@WenInCode
Copy link
Contributor Author

Going to update this with relationship helpers from #547 once they get merged.


assert.ok(attributes.includes('description'), 'should have description attribute');
assert.ok(attributes.includes('name'), 'should have name attribute');
assert.ok(attributes.includes('slug'), 'should have slug attribute');
Copy link
Contributor

Choose a reason for hiding this comment

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

We should refactor these into an assert.hasAttribute(attributes, 'attribute') which itself just runs assert.ok() with the description we use there.

Let's make an issue to circle back there. Will DRY these up slightly.

Copy link
Contributor

@joshsmith joshsmith left a comment

Choose a reason for hiding this comment

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

👍 good but let's open that issue.

@WenInCode WenInCode merged commit d7fe8d4 into code-corps:develop Oct 2, 2016
@WenInCode WenInCode deleted the update-category-model-tests branch October 2, 2016 20:45
WenInCode added a commit that referenced this pull request Jan 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants