Skip to content

add ddt test dependency (seems required for test suite to pass with current git-annex)#17

Closed
notestaff wants to merge 3 commits intoconda-forge:masterfrom
notestaff:is-add-ddt-test-dependency
Closed

add ddt test dependency (seems required for test suite to pass with current git-annex)#17
notestaff wants to merge 3 commits intoconda-forge:masterfrom
notestaff:is-add-ddt-test-dependency

Conversation

@notestaff
Copy link
Copy Markdown
Contributor

Checklist

  • Used a fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

@notestaff notestaff requested a review from yarikoptic as a code owner August 20, 2019 02:52
@notestaff notestaff self-assigned this Aug 20, 2019
@conda-forge-linter
Copy link
Copy Markdown

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@notestaff
Copy link
Copy Markdown
Contributor Author

P.S. I've dropped the 'datalad test' command from git-annex tests, because I'm not sure such a dependency is right by conda-forge guidelines, and because datalad has a fair number of test dependencies that would clutter the git-annex recipe. Instead, when a new git-annex comes out I'll make a datalad PR (which you don't have to merge) to test that the new version still works with datalad.

@yarikoptic
Copy link
Copy Markdown
Contributor

yeap, makes sense. Thanks @notestaff !

@notestaff
Copy link
Copy Markdown
Contributor Author

(this PR might still be worth merging, as it adds a missing test dependency, and includes a rerender)

@yarikoptic
Copy link
Copy Markdown
Contributor

yarikoptic commented Aug 20, 2019

I am confused a bit now -- we aren't using ddt in datalad tests. Why is it needed? azure pipelines seems to be all green: https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=58345

@notestaff
Copy link
Copy Markdown
Contributor Author

I was getting error messages that GitPython requires ddt, when running the tests locally.

@yarikoptic
Copy link
Copy Markdown
Contributor

it seems that GitPython itself does use/require it for tests, and even has it listed in requirements.txt, which populates `install_requires` (do not see direct need for that but I guess there is). So if anywhere -- the fix (adding ddt) should go against GitPython conda package not datalad.
(git)hopa:~deb/python-git[upstream-master]git
$> git grep ddt
git/test/test_diff.py:import ddt
git/test/test_diff.py:@ddt.ddt
git/test/test_diff.py:    @ddt.data(
git/test/test_exc.py:import ddt
git/test/test_exc.py:@ddt.ddt
git/test/test_exc.py:    @ddt.data(*list(itt.product(_cmd_argvs, _causes_n_substrings, _streams_n_substrings)))
git/test/test_exc.py:    @ddt.data(
git/test/test_exc.py:    @ddt.data(
git/test/test_exc.py:    @ddt.data(
git/test/test_util.py:import ddt
git/test/test_util.py:@ddt.ddt
git/test/test_util.py:    @ddt.idata(_norm_cygpath_pairs + _unc_cygpath_pairs)
git/test/test_util.py:    @ddt.data(
git/test/test_util.py:    @ddt.data(
git/test/test_util.py:    @ddt.idata(_norm_cygpath_pairs)
git/test/test_util.py:    @ddt.data(('name', ''), ('name', 'prefix_'))
requirements.txt:ddt>=1.1.1
test-requirements.txt:ddt>=1.1.1

@notestaff
Copy link
Copy Markdown
Contributor Author

So, somehow GitPython tests were getting triggered by 'datalad test'?

@yarikoptic
Copy link
Copy Markdown
Contributor

depending on how actual error/traceback looked like, it could have been just setuptools whining that the requirement ddt is not installed. (you don't even need to try importing it). But also seeing traceback could provide information on either it might be nose indeed trying to find the tests within git. Probably anyways the best way to invoke datalad tests would be python -m nose datalad

@yarikoptic
Copy link
Copy Markdown
Contributor

trying to replicate now. observation 1: indeed conda install gitpython does not pull in `ddt`
hopa:~/anaconda-2018.12-3.7/bin
$> conda install gitpython 
Solving environment: done


==> WARNING: A newer version of conda exists. <==
  current version: 4.5.12
  latest version: 4.7.11

Please update conda by running

    $ conda update -n base -c defaults conda



## Package Plan ##

  environment location: /home/yoh/anaconda-2018.12-3.7/envs/test-gitpython

  added / updated specs: 
    - gitpython


The following packages will be downloaded:

    package                    |            build
    ---------------------------|-----------------
    libstdcxx-ng-9.1.0         |       hdf63c60_0         4.0 MB
    openssl-1.1.1c             |       h516909a_0         2.1 MB  conda-forge
    libgcc-ng-9.1.0            |       hdf63c60_0         8.1 MB
    ca-certificates-2019.6.16  |       hecc5488_0         145 KB  conda-forge
    python-3.7.3               |       h33d41f4_1        36.0 MB  conda-forge
    readline-8.0               |       hf8c457e_0         441 KB  conda-forge
    gitpython-3.0.1            |             py_0         342 KB  conda-forge
    zlib-1.2.11                |    h516909a_1005         105 KB  conda-forge
    setuptools-41.2.0          |           py37_0         635 KB  conda-forge
    sqlite-3.29.0              |       hcee41ef_0         2.0 MB  conda-forge
    tk-8.6.9                   |    hed695b0_1002         3.2 MB  conda-forge
    _libgcc_mutex-0.1          |             main           3 KB
    pip-19.2.2                 |           py37_0         1.9 MB  conda-forge
    certifi-2019.6.16          |           py37_1         149 KB  conda-forge
    libffi-3.2.1               |    he1b5a44_1006          46 KB  conda-forge
    bzip2-1.0.8                |       h516909a_0         396 KB  conda-forge
    wheel-0.33.6               |           py37_0          35 KB  conda-forge
    ------------------------------------------------------------
                                           Total:        59.5 MB

The following NEW packages will be INSTALLED:

    _libgcc_mutex:   0.1-main                        
    bzip2:           1.0.8-h516909a_0     conda-forge
    ca-certificates: 2019.6.16-hecc5488_0 conda-forge
    certifi:         2019.6.16-py37_1     conda-forge
    gitdb2:          2.0.5-py_0           conda-forge
    gitpython:       3.0.1-py_0           conda-forge
    libffi:          3.2.1-he1b5a44_1006  conda-forge
    libgcc-ng:       9.1.0-hdf63c60_0                
    libstdcxx-ng:    9.1.0-hdf63c60_0                
    ncurses:         6.1-hf484d3e_1002    conda-forge
    openssl:         1.1.1c-h516909a_0    conda-forge
    pip:             19.2.2-py37_0        conda-forge
    python:          3.7.3-h33d41f4_1     conda-forge
    readline:        8.0-hf8c457e_0       conda-forge
    setuptools:      41.2.0-py37_0        conda-forge
    smmap2:          2.0.5-py_0           conda-forge
    sqlite:          3.29.0-hcee41ef_0    conda-forge
    tk:              8.6.9-hed695b0_1002  conda-forge
    wheel:           0.33.6-py37_0        conda-forge
    xz:              5.2.4-h14c3975_1001  conda-forge
    zlib:            1.2.11-h516909a_1005 conda-forge

Proceed ([y]/n)? y


Downloading and Extracting Packages
libstdcxx-ng-9.1.0   | 4.0 MB    | ##################################################################### | 100% 
openssl-1.1.1c       | 2.1 MB    | ######...

@yarikoptic
Copy link
Copy Markdown
Contributor

ok, reproduced!

======================================================================
ERROR: datalad.metadata.tests.test_aggregation.test_update_strategy
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/yoh/anaconda-2018.12-3.7/envs/test-gitpython/lib/python3.7/site-packages/datalad/metadata/metadata.py", line 511, in _get_metadata
    extractor_cls = extractors[mtype_key].load()
  File "/home/yoh/anaconda-2018.12-3.7/envs/test-gitpython/lib/python3.7/site-packages/pkg_resources/__init__.py", line 2442, in load
    self.require(*args, **kwargs)
  File "/home/yoh/anaconda-2018.12-3.7/envs/test-gitpython/lib/python3.7/site-packages/pkg_resources/__init__.py", line 2465, in require
    items = working_set.resolve(reqs, env, installer, extras=self.extras)
  File "/home/yoh/anaconda-2018.12-3.7/envs/test-gitpython/lib/python3.7/site-packages/pkg_resources/__init__.py", line 786, in resolve
    raise DistributionNotFound(req, requirers)
pkg_resources.DistributionNotFound: The 'ddt>=1.1.1' distribution was not found and is required by GitPython

so as I have feared -- ddt is probably not really needed for runtime but because it is part of install_requires -- here is the kaboom. It was added in GitPython only on Jul 20 so we didn't see that before: gitpython-developers/GitPython@74a0507

I have submitted a PR to GitPython upstream to have ddt removed from requirements.txt: gitpython-developers/GitPython#911
Not sure if conda's package for it should just wait or patch it away meanwhile or add not really needed ddt requirement. But I think we should not add it here in datalad, thus closing

@yarikoptic yarikoptic closed this Aug 21, 2019
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.

3 participants