simphony / simphony-mayavi

The mayavi adapters to the simphony framework
BSD 2-Clause "Simplified" License
0 stars 1 forks source link

mock is required for testing #113

Closed kitchoi closed 8 years ago

kitchoi commented 8 years ago

@dpinte Only dev-requirements.txt is updated.

itziakos commented 8 years ago

i do not see where nose is used?

itziakos commented 8 years ago

I also do not see where mock is used for the tests?

kitchoi commented 8 years ago

nose is required by from traitsui.tests._tools import is_current_backend_qt4. Qt4 is required by ModalDialogTester in test_show and others. Because simphony_mayavi.show would still work even if the backend is not qt4, I think it is more sensible to skip the tests than failing them. In addition to test_show, simphony_mayavi.plugins.tests.test_add_source_panel etc. also require knowledge about the backend, so from traitsui.tests._tools import is_current_backend_qt4 (i.e. nose) is required nonetheless.

For mock, ModalDialogTester requires GuiTestAssistant, which imports mock. test_show needs it, and a number of tests for the new panels in simphony_mayavi.plugins require ModalDialogTester too. As far as I can tell, mock is required by version 0.3.1 already.

itziakos commented 8 years ago

nose is required by from traitsui.tests._tools import is_current_backend_qt4

is_current_backend_qt4 does not need nose. Nose is needed only for skip_if_not_backend. I would prefer that we copy the is_current_backend_qt4 related code in simphony mayavi (especially when _tool is a private module of traitsui).

For mock, ModalDialogTester requires GuiTestAssistant, which imports mock.

yup, I just saw that.

Please make sure that we update the documentation on testing requirements. I also saw that mock is not needed anymore for creating the documentation correct?

kitchoi commented 8 years ago

I also saw that mock is not needed anymore for creating the documentation correct?

That's correct.

Please make sure that we update the documentation on testing requirements.

Thanks for pointing that out. Will do.

kitchoi commented 8 years ago

@itziakos, @dpinte, I modifed README.rst too. I also took the opportunity to mention ETS_TOOLKIT=qt4 (can be considered as a requirement?!)

kitchoi commented 8 years ago

I will investigate porting is_current_backend_qt4 tomorrow.

itziakos commented 8 years ago

can be considered as a requirement?!

I would consider it a soft requirement. Mayavi is supposed to work with both qt and wx. It is that we mainly tests using qt that makes the need for qt implicit and that wx does not get the same type of support.

kitchoi commented 8 years ago

@itziakos, @dpinte Changed to use traits.etsconfig.ETSConfig to check for the backend. nose is no longer required. I believe the files I change here still won't have any conflict with the other PRs, testing...

kitchoi commented 8 years ago

@dpinte, is this good to merge?

dpinte commented 8 years ago

@kitchoi LGTM