typemytype / booleanOperations

Boolean operations on paths
MIT License
39 stars 18 forks source link

adding test #46

Closed typemytype closed 6 years ago

typemytype commented 6 years ago

and maybe those test could happen on travis...

typemytype commented 6 years ago

and those tests could be a start point for other clipping libraries

anthrotype commented 6 years ago

Thank you Frederik!

The tests are already running on Travis, the unittest-style tests are automatically discovered by pytest, and run by tox inside a virtual environment. They fail because one file is attempting to import from mojo (RF-only) and another from fontPens, which is not specified as a test requirement.

What is that generateGlyphDataRoboFont.py? Is it part of the test suite itself or rather something "meta" that is not required to run the tests, but to actually create the test data? If so, we should move it outside of the Lib itself.

The missing fontPens can be added as a test requirement in the tox.ini file (under deps, just after pytest).. oh but for that we first need to publish fontPens to PyPI. I'll do that.

typemytype commented 6 years ago

that generateGlyphDataRoboFont.py file is just a helper file to generate the test data ufo from the main layer and visually check the excepted result in RoboFont (the visual check has to happen manually) but this is already a big step!

anthrotype commented 6 years ago

there's an open issue about fontPens.digestPointPen https://github.com/robofab-developers/fontPens/issues/12, I guess it's better to wait that before tagging the first fontPens release on PyPI? Maybe you can simply copy a DigestPointPen class in here for the time being, if it's the only thing that you need fontPens for.

If you want to keep that generateGlyphDataRoboFont module in the lib (so that you can run it from within RF) then you need to move the top-level mojo import inside a nested scope, i.e. in the function that uses it, so it is delayed and will only fail if one actually attempts to run the function, which should not happen in the Travis test run.

typemytype commented 6 years ago

does pytest auto discover tests outside the lib?

anthrotype commented 6 years ago

does pytest auto discover tests outside the lib?

yes, it does. It currently searches in Lib/booleanOperations and in an external tests/ folder. See the testpaths key in setup.cfg: https://github.com/typemytype/booleanOperations/blob/master/setup.cfg#L12

In general I prefer to have the test suite in an external directory instead of inside the library, because it is not necessary at runtime for normal users, only for developers. And pytest can discover external tests just fine. The reason I put Lib/booleanOperations in the pytest's testpaths is to be able to discover inline doctests (if any).

typemytype commented 6 years ago

its now outside the lib

but it seem not to be finding the ufo data...

typemytype commented 6 years ago

I dont get InvocationError: '/home/travis/build/typemytype/booleanOperations/.tox/py27/bin/pytest'

anthrotype commented 6 years ago

By default, pytest will consider any file matching with test_.py and _test.py globs as a test module

https://docs.pytest.org/en/latest/customize.html

you can either rename the testBooleanGlyph.py module to something pytest can find, or you can tell pytest to consider test modules all the files that, e.g. start with test*.py, so you keep the current name without the separating underscore.

You need to add this to the setup.cfg, then it works (tested locally on my linux machine):

diff --git a/setup.cfg b/setup.cfg
index fc112fd..c00e98a 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -12,6 +12,8 @@ minversion = 3.0.2
 testpaths =
     Lib/booleanOperations
     tests
+python_files =
+       test*.py
 addopts =
     # run py.test in verbose mode
     -v
typemytype commented 6 years ago

oh thanks, this feels magically!

typemytype commented 6 years ago

will merge it when fontPens is in PyPi so the digestPointPen can be removed

good to have some test and a platform to easy add tests

anthrotype commented 6 years ago

@typemytype can we merge this now? sorry I let this go..

typemytype commented 6 years ago

@anthrotype euh these are weird errors, I guess

https://ci.appveyor.com/project/typemytype/booleanoperations/build/1.0.120

anthrotype commented 6 years ago

fixed, thanks