test failures in suite; version -0.7.2; FAILED (failures=2)

[idella]

According to issue 13 the tests might be a relatively recent addition.

I found the .travis.yml and adjusted the running to what I had and ended with the same result.
PYTHONPATH=.:cytoolz- nosetests --with-doctest cytoolz

from the source and from an ebuild.

======================================================================
FAIL: test_curried_toolzlike.test_toolz_like_cytoolz
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib64/python3.4/site-packages/nose/case.py", line 198, in runTest
    self.test(*self.arg)
  File "/mnt/gen2/TmpDir/portage/dev-python/cytoolz-0.7.2/work/cytoolz-0.7.2-python3_4/lib/cytoolz/tests/test_curried_toolzlike.py", line 35, in test_toolz_like_cytoolz
    'cytoolz.curried.%s should not exist' % key)
AssertionError: cytoolz.curried.itemfilter should not exist

======================================================================
FAIL: test_docstrings.test_docstrings_uptodate
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib64/python3.4/site-packages/nose/case.py", line 198, in runTest
    self.test(*self.arg)
  File "/mnt/gen2/TmpDir/portage/dev-python/cytoolz-0.7.2/work/cytoolz-0.7.2-python3_4/lib/cytoolz/tests/test_docstrings.py", line 72, in test_docstrings_uptodate
    key, '\n'.join(fulldiff))
AssertionError: Error: cytoolz.drop has a bad docstring:
+ drop(Py_ssize_t n, seq)
+ 
-  The sequence following the first n elements
+     The sequence following the first n elements
? +++


      >>> list(drop(2, [10, 20, 30, 40, 50]))
      [30, 40, 50]
+ 
+     See Also:
+         take
+         tail



----------------------------------------------------------------------
Ran 150 tests in 0.282s

FAILED (failures=2)

under pythons 2.7 3.3 3.4.
Do you require any further info?

[eriknw]

Thanks for the report. What version of toolz do you have installed?

The docstring error is odd. It passes travis-ci for python 2.6, 2.7, 3.3, and 3.4. However, when I run it on my machine, it fails in Python 3.4, but works in 2.7. What's even weirder, though, is that drop doesn't fail for me (it passes the test just fine), but groupby fails.

I'll investigate further.

Also, to run the tests, you can simply do make test if you have cloned the repo. The tests are not a new addition, we were just wondering if there's a better way to test both toolz and cytoolz without copying the tests from toolz to cytoolz. In practice, this works just fine.

[eriknw]

Okay, these tests failures are benign.

These particular tests are only supposed to be run before a new version of cytoolz is released to PyPI. They must be run against a specific version of toolz (specifically, cytoolz.__toolz_version__). The purpose of these tests is to ensure that cytoolz is like toolz, which includes docstrings, namespaces, and what gets curried in *toolz.curried. I was getting an error with groupby, because the docstring changed slightly in the dev version of toolz.

These tests get skipped if 'dev' is in cytoolz.__version__.

Let me know if you have any questions or concerns. I will close this issue in a day or so if no response.

[idella]

hmm, you mean it's version sensitive??

dev-python/toolz
Available versions: (~)0.7.0 0.7.1 {PYTHON_TARGETS="python2_7 python3_3 python3_4"}
Installed versions: 0.7.0(11:55:57 28/10/14)

Normally I rely upon a version sensitivity to be specified, then I can simply set it and all is well.
If this relies upon a matching 0.7.2 I would have to first bump toolz and system install it.

At this point I'd like reply to above. I can either exclude these tests on the basis that you the author declare them "Okay, these tests failures are benign." OR get the right versions set and have them all pass. Either 'works'. Thx for reply

[eriknw]

Yes, it's version-specific. The latest cytoolz release was tested on TravisCI using this script:

https://github.com/eriknw/cytoolz/blob/check_my_c_files/.travis.yml

and with the results here:

https://travis-ci.org/eriknw/cytoolz/builds/52994658

It was recently suggested that we have--and use, tag, etc.--a "release" branch in the pytoolz/cytoolz repo. I will do this for the next release. This branch uses a slightly modified .travis.yaml file and includes Cython-generated C files.

We can skip the release tests if the version of toolz doesn't match the version cytoolz expects. For example, we could add if toolz.__version__ == cytoolz.__toolz_version__ check here:

https://github.com/pytoolz/cytoolz/blob/master/cytoolz/tests/dev_skip_test.py

Regarding how you should proceed: you may update toolz to 0.7.1, or you may ignore these tests, because I declare them to be benign :)

[idella]

@erlknw; yes, either work

we could add if toolz.version == cytoolz.toolz_version check here:

would be good for future releases, but really, "because I declare them to be benign " is quite sufficient for me for now. As long as it is dealt with by an author, 'we' come to a decision accordingly.

oh remember that a '*dev' version is not one generally embraced in gentoo. Just give us version release. Also, Travis is an optional testing system not used in a test phase in an ebuild. Here, It's merely a testing house used by upstream in preparing a testsuite before releasing it in a tagged version in a tarball.

[eriknw]

I'm glad I was able to clarify things! Thanks for raising the issue. I will make changes to avoid this type of confusion in the future.

Is there anything you would like done differently in order for toolz and cytoolz to play nicely with gentoo and ebuild? We use a "*dev" version between so-called official releases, which are uploaded to PyPI. There is generally no harm in using the latest code from the master branch on github if that's your preference. However, there is a difference between dev versions and official releases: dev versions require Cython, but official releases already include C files and only require a C compiler.

[idella]

@eriknw , thx for your offer to line up with gentoo. I wish more could be like you re this.

I just set a >= border version it the ebuild, emerged the dev-python/toolz-0.7.1 and all tests passed so all is good there. What I did notice though is "cytoolz is an implementation of the toolz package" which means that toolz is a run time dep. In the ebuild this was actually absent. I have ofc added it. Sure enough I find toolz is imported in the core module(s) of cytoolz.

While it may be obvious (to the seasoned), what we rely on in is a stipulation in setup.py often achieved with install_requires=[package]. Sometimes authors add a requires.txt file somewhere in the source, then sometimes a file tests-require.txt etc.

Currently I run the suite with
PYTHONPATH=.:${PN} nosetests --with-doctest ${PN} || die "tests failed under ${EPYTHON}"
which ofc have inhouse vars of gentoo but it's easy to see what it does. I totally forgot about make test so I must try it some time. This is purely force of habit. make test is merely rarely used in pythonic packages to invoke the test run.

[eriknw]

Quick note: toolz is not a runtime dependency of cytoolz. Where do you see this? It is, however, a developmental dependency to perform the toolz-cytoolz comparison tests. Is this enough for you to consider it a dependency?

[idella]

/mnt/gen2/TmpDir/portage/dev-python/cytoolz-0.7.2-r2/work/cytoolz-0.7.2/cytoolz/curried.py:# import toolz

So it's not a runtime dep, a build dep? Is curried re only the tests making it a dep of tests? Looks it. There's also a cmd tests_require=[ ]

examples;
flake8-2.4.0/work/flake8-2.4.0/setup.py: tests_require += ['mock']
Beaker-1.6.4/setup.py:tests_require = ['nose', 'webtest', 'Mock']
Beaker-1.6.4/setup.py: tests_require=tests_require,

he makes a var tests_require=[ } then later
tests_require=tests_require,

[eriknw]

Just to clarify, toolz is never imported by cytoolz except during tests that should only be run by me (or other developers). These tests do not test the core functionality of cytoolz and may be skipped. toolz and cytoolz actually have very extensive tests.

The line you mentioned
https://github.com/pytoolz/cytoolz/blob/master/cytoolz/curried.py#L35
is a comment.

toolz is not a dependency of cytoolz. #58 makes it explicit that toolz does not need to be installed to test any version (dev or release) of cytoolz, and if toolz is installed, the version of toolz does not matter.

Regarding developmental/testing requirements, there is a file: requirements_devel.txt. Releases of cytoolz (those posted to https://pypi.python.org/pypi/cytoolz) only require a C compiler. If you use the bleeding edge cytoolz from github, you will need Cython as well.

[idella]

Hmmmmm /cytoolz-0.7.2/cytoolz/curried.py
it is a comment.

"release tests if the version of toolz doesn't match the version"
and
"you may update toolz to 0.7.1"
says it is in fact needed for the run of the tests for this version. These other fine tunings will make it more robust / change it in future releases. For now, I'll take it out as an rdep and make it a dep for the tests.

[eriknw]

Cool, so I think we've addressed your immediate issue, and I've made changes that will hopefully avoid any such confusion in the future. Now, cytoolz does not require nose or toolz when running tests (but you still should use a test runner, such as py.test).

I think the final action item from this thread is how to version *toolz to play nicely with gentoo/ebuild. Can you educate me?

[idella]

eeer, I thought we had but what comes to mind at the moment is simply to have a border version clearly set in setup.py.

< does not require nose or toolz when running tests>
well I'm not sure how I ended up with nose but it works so I think leaving it as is in the ebuild is fine considering you say (but you still should use a test runner,)

This

        test? ( dev-python/nose[${PYTHON_USEDEP}]
                >=dev-python/toolz-0.7.1[${PYTHON_USEDEP}] )"

is what I have for the test phase in the ebuild for now however we have a contradiction. While you say that cytoolz does not require toolz when running tests, you ask how to get it to play nicely in an ebuild. I thought that you were setting tests to skip reading the toolz version if toolz was either absent or a versioned mis-match. I was expecting to drop
dev-python/toolz-0.7.1[${PYTHON_USEDEP}]
all together in the next bump on that basis. By rights there shouldn't be a dep set in DEPEND RDEPEND that isn't legit even though it won't hurt, it's just a passenger.

The generic answer to the basic q from my stance is that a required version for the package is normally set in setup.py in install_requires=[ ] or tests_require(s)=[ ]. There are some other hybrids. These I will always check for and we take them as gospel and add them to the ebuild as they are written. On rare occasions we might find they aren't correct and file accordingly.

Does this answer?.