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

Tests: Set individual tests as TODO, not modules. #12768

Merged
merged 3 commits into from Nov 28, 2017

Conversation

donmccurdy
Copy link
Collaborator

Fixes #12714, fixes #12759.

Also removes TODOs on private methods (or the ones that I found, anyway...).

result
before qunit_before
after qunit_after

@donmccurdy
Copy link
Collaborator Author

/cc @Itee

@mrdoob mrdoob merged commit 311f502 into mrdoob:dev Nov 28, 2017
@mrdoob
Copy link
Owner

mrdoob commented Nov 28, 2017

Thanks!

@Itee
Copy link
Contributor

Itee commented Nov 30, 2017

It is better to mark todo in the right place.

But remove the test about private methods is an error i think !
Currently we are unable to prove that private methods are correct ! And IMO they should be tested harder than public interface.

@donmccurdy
Copy link
Collaborator Author

Currently we are unable to prove that private methods are correct ! And IMO they should be tested harder than public interface.

It's OK for people to contribute tests of private methods when they're helpful, but in general I do not think all private methods should have tests. See discussion here. It is better to test what happens, rather than how it happens, and private methods are often the latter.

Certainly it is fine for tests on private methods to be written — I have no objection to that — but I would rather add those tests only when/if they make sense, and not to have TODOs complaining when they aren't there.

@Itee
Copy link
Contributor

Itee commented Dec 1, 2017

Ok i understand your opinion !

But like said in comment:

I disagree. Ideally, you write a quick test before you start coding a function. Think of typical input and what the output will be. Write the test (which shouldn't take you longer than a few seconds) and code until it gets the test right. There is no reason to abandon that style of work for private methods. – Frank

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.

Unit tests status - what to do with unit test bugs / mistakes? Tests that should be passing are marked TODO
3 participants