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

Refactor test #47

Merged
merged 4 commits into from
Aug 29, 2019
Merged

Refactor test #47

merged 4 commits into from
Aug 29, 2019

Conversation

segayuu
Copy link
Contributor

@segayuu segayuu commented Jul 3, 2019

No description provided.

@segayuu segayuu requested a review from a team July 3, 2019 03:23
@tcrowe
Copy link
Contributor

tcrowe commented Jul 3, 2019

Haaay @segayuu 👋

I'd like to help out with this. Maybe you can do it or I can do it.


We can improve ./test/mocha.opts
https://gist.github.com/tcrowe/b22a63daea19eaee69fe2bc37adee854

require('chai').should(); can be removed if you --require should in ./test/mocha.opts


https://github.com/hexojs/warehouse/pull/47/files#diff-659252ed9705c67ef45fa76b7758f9d8R88

.should.to. can just be .should.and I don't think .to. is needed!


Was this helpful?

@segayuu
Copy link
Contributor Author

segayuu commented Jul 4, 2019

Thank @tcrowe !

We can improve ./test/mocha.opts

The omission of require ('chai').should() by ./test/mocha.opts is not my preference as it destroys type inference.
On the other hand, because there are many test files, it may be correct to give preference to omitting them.


.should.to. can just be .should. and I don't think .to. is needed!

I personally agree, but if that is the case, it would remove the advantage that the Should-style "code composes English".
It should be uniform across all assert lines as it relates to the coding policy.
Since PR will grow to the entire test code, I think it will be a single PR even if it is done. I do not intend to do this in this PR.

@segayuu segayuu requested review from a team and removed request for a team August 27, 2019 03:32
@segayuu segayuu merged commit 84e5869 into hexojs:master Aug 29, 2019
@segayuu segayuu deleted the Refactor-Test branch August 29, 2019 14:06
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

Successfully merging this pull request may close these issues.

3 participants