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

Assertion Verbs #865

Closed
CfGit12 opened this issue Apr 13, 2021 · 2 comments
Closed

Assertion Verbs #865

CfGit12 opened this issue Apr 13, 2021 · 2 comments
Assignees
Milestone

Comments

@CfGit12
Copy link

CfGit12 commented Apr 13, 2021

Disclaimer: I'm aware I can customise the assertion verbs, but generally speaking I'd not expect to have to do this without exceptional circumstances.

I find the out of the box assertion verbs to be a little bit muddled. I'm going to give a few examples of both expect and assertThat to illustrate my point. Firstly expect:

expect(x).toBe(y) makes sense and reads well.

expect(x).isLessThan(y) does not read well. I would expect expect(x).toBeLessThan(y).

expect(x).containsOnly(y) isn't as bad but is still a bit iffy. Would prefer expect(x).toContainOnly(y).

expect {}.toThrow reads well.

Using the assertThat notation we have:

assertThat(x).toBe(y) which doesn't read well at all. I would expect isEqualTo to be included for this case.

assertThat(x).isLessThan(y) on the other hand does read well in this case.

assertThat(x).containsOny(y) reads well.

assertThat {}.toThrow does not read well.

Essentially out of the box both expect and assertThat have problems. The assertions themselves feel like they're not written consistently for the assertion prefix they're expecting.

I don't have any particular preference over expect and assertThat but as it stands each of them have issues. When I come to demo this library to my team this will be the first thing they see and for me it sticks out a bit.

@robstoll
Copy link
Owner

robstoll commented Apr 13, 2021

@CfGit12 thanks for your feedback, very much appreciated.

expect(x).isLessThan(y) does not read well. I would expect expect(x).toBeLessThan(y).

We are going to rewrite the API with the up-coming version to a consistent to + infinitive form, i.e. isLessThan well be renamed to toBeLessThan as you suggest. You can read more about it and give further feedback/ideas here robstoll/atrium-roadmap#93. For instance, one open point. shall we provide only toBeA as replacement of isA or should we have toBeAn additionally or use a totally different name?

That said, maybe we should remove assertThat as predefined assertion-verb as it will not read nice as you pointed out. I think assert(x).toBeLessThan(y) still reads fine but maybe it makes more sense to have only one assertion verb?

Last but not least, let me know in case you have free capacities and could help to rename stuff, we could use your help.

@CfGit12
Copy link
Author

CfGit12 commented Apr 13, 2021

Excellent :)

Posted a couple of thoughts in the other issue.

I don't really have any availability to directly contribute I'm afraid but happy to provide feedback when required.

@CfGit12 CfGit12 closed this as completed Apr 13, 2021
@robstoll robstoll added this to the 0.17.0 milestone Oct 14, 2021
@robstoll robstoll self-assigned this Oct 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants