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

adding test #46

Merged
merged 14 commits into from
Jul 24, 2018
Merged

adding test #46

merged 14 commits into from
Jul 24, 2018

Conversation

typemytype
Copy link
Owner

No description provided.

@typemytype
Copy link
Owner Author

and maybe those test could happen on travis...

@typemytype
Copy link
Owner Author

typemytype commented Jan 25, 2018

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

@anthrotype
Copy link
Collaborator

anthrotype commented Jan 25, 2018

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
Copy link
Owner Author

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
Copy link
Collaborator

there's an open issue about fontPens.digestPointPen robotools/fontPens#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
Copy link
Owner Author

does pytest auto discover tests outside the lib?

@anthrotype
Copy link
Collaborator

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
Copy link
Owner Author

its now outside the lib

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



def _addGlyphTests():
root = os.path.join(os.path.dirname(__file__), 'testdata')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/testdata/testData/

@typemytype
Copy link
Owner Author

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

@anthrotype
Copy link
Collaborator

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
Copy link
Owner Author

oh thanks, this feels magically!

Copy link
Collaborator

@anthrotype anthrotype left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@typemytype
Copy link
Owner Author

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
Copy link
Collaborator

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

@typemytype typemytype merged commit 24d41bf into master Jul 24, 2018
@typemytype
Copy link
Owner Author

@anthrotype anthrotype deleted the addingTest branch July 25, 2018 10:11
@anthrotype
Copy link
Collaborator

fixed, thanks

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