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

Public method which allows to clean 3Display #807

Merged
merged 2 commits into from
May 7, 2014

Conversation

copyme
Copy link
Member

@copyme copyme commented Apr 29, 2014

This commit is related to the issue #806.

@dcoeurjo
Copy link
Member

dcoeurjo commented May 3, 2014

HI @copyme, thanks for the PR and sorry for not being very active this week (out of my office). To complete your PR, could you please:

  • add a line in the Changelog about your feature
  • add a new function which tests this feature (in tests/io/boards/testBoard3d for instance)
  • add a new line in the doc about that

For the last item, if you have any trouble with doxygen, I'll do it by myself ;)

Thanks again

@copyme copyme changed the title Poblic method which allows to clean 3Display Public method which allows to clean 3Display May 3, 2014
@copyme
Copy link
Member Author

copyme commented May 3, 2014

@dcoeurjo Just a question about tests. As I see some of them just running methods of given class without really testing something in some sense. For me test should be automatic and autonomous - eg not touching any external things like hard drive or screen. (I do agree this is in many cases impossible but) Right now I see that some tests in DGtal hardly depend on person which will run these tests and take a look on result like files or image on screen and thus they are neither automatic nor autonomous.

In this case I want to test clear() so normally I will create a mock/fake object which implements Display3D which "try to" display something and gives me info e.g. how many object in scene I have before and after calling clear(). I have created a test which is testing clear() by using Board3D and saving files on hard drive and after that I call clear() and again another type of objects are send and save to another file. But to say if test really passed or not I have take a look on result. So this is not really a test in some sense. Sorry if I am wrong, but just to say I am new here ;)

@dcoeurjo
Copy link
Member

dcoeurjo commented May 4, 2014

You are completely right, unit tests associated with a feature must assert something as close as possible to the semantic attached to this feature. And I also agree that it does not make sense for io stuff (excerpt with board2d or board3d).

In the DGtal test framework, we try to do as clever as possible unit tests. If it is not feasible, we just test some API (not really unit test anymore but useful for regression tests). For interactive things (Viewre3d), the test is just compiled (for API checks) but not included in the make test command.

For the clear feature, maybe you can still do something with Board3d: if we add a function which returns the sizes of the internal vectors (number of quads... Which could be useful btw) you could assert that clearing the boards set everything to 0. I agree that's a bit weird but as you said, that's all we can do with this ;)

@dcoeurjo
Copy link
Member

dcoeurjo commented May 7, 2014

Thanks a lot.. merging (maybe in there will be conflicts with #802). I'll add a changlog entry (that needs to be refactor).

dcoeurjo added a commit that referenced this pull request May 7, 2014
Public method which allows to clean 3Display
@dcoeurjo dcoeurjo merged commit 49c4c1e into DGtal-team:master May 7, 2014
@dcoeurjo
Copy link
Member

dcoeurjo commented May 7, 2014

congrats for your first PR !

@copyme copyme deleted the 3Display-cleaning branch June 26, 2014 08:28
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.

2 participants