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

2.x release doesn't pass tests #164

Closed
alerque opened this issue Nov 10, 2020 · 14 comments
Closed

2.x release doesn't pass tests #164

alerque opened this issue Nov 10, 2020 · 14 comments

Comments

@alerque
Copy link
Contributor

alerque commented Nov 10, 2020

I just tried to update the Arch Linux package for panflute and ran into trouble with the check step which runs python setup.py test. Against Pandoc 2.11.0.2 it's throwing errors like this:

==> Starting check()...
running test
WARNING: Testing via this command is deprecated and will be removed in a future version. Users looking for a generic test entry point independent of test runner are encouraged to use tox.
running egg_info
writing panflute.egg-info/PKG-INFO
writing dependency_links to panflute.egg-info/dependency_links.txt
writing entry points to panflute.egg-info/entry_points.txt
writing requirements to panflute.egg-info/requires.txt
writing top-level names to panflute.egg-info/top_level.txt
reading manifest file 'panflute.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
writing manifest file 'panflute.egg-info/SOURCES.txt'
running build_ext
Traceback (most recent call last):
  File "setup.py", line 22, in <module>
    setup(
  File "/usr/lib/python3.8/site-packages/setuptools/__init__.py", line 153, in setup
    return distutils.core.setup(**attrs)
  File "/usr/lib/python3.8/distutils/core.py", line 148, in setup
    dist.run_commands()
  File "/usr/lib/python3.8/distutils/dist.py", line 966, in run_commands
    self.run_command(cmd)
  File "/usr/lib/python3.8/distutils/dist.py", line 985, in run_command
    cmd_obj.run()
  File "/usr/lib/python3.8/site-packages/setuptools/command/test.py", line 232, in run
    self.run_tests()
  File "/usr/lib/python3.8/site-packages/setuptools/command/test.py", line 250, in run_tests
    test = unittest.main(
  File "/usr/lib/python3.8/unittest/main.py", line 100, in __init__
    self.parseArgs(argv)
  File "/usr/lib/python3.8/unittest/main.py", line 124, in parseArgs
    self._do_discovery(argv[2:])
  File "/usr/lib/python3.8/unittest/main.py", line 244, in _do_discovery
    self.createTests(from_discovery=True, Loader=Loader)
  File "/usr/lib/python3.8/unittest/main.py", line 154, in createTests
    self.test = loader.discover(self.start, self.pattern, self.top)
  File "/usr/lib/python3.8/unittest/loader.py", line 349, in discover
    tests = list(self._find_tests(start_dir, pattern))
  File "/usr/lib/python3.8/unittest/loader.py", line 405, in _find_tests
    tests, should_recurse = self._find_test_path(
  File "/usr/lib/python3.8/unittest/loader.py", line 483, in _find_test_path
    tests = self.loadTestsFromModule(package, pattern=pattern)
  File "/usr/lib/python3.8/site-packages/setuptools/command/test.py", line 50, in loadTestsFromModule
    tests.append(self.loadTestsFromName(submodule))
  File "/usr/lib/python3.8/unittest/loader.py", line 154, in loadTestsFromName
    module = __import__(module_name)
  File "/home/caleb/projects/aur/python-panflute/src/panflute-2.0.4/tests/quick.py", line 6, in <module>
    doc = Doc(x, metadata=m)
  File "/home/caleb/projects/aur/python-panflute/src/panflute-2.0.4/panflute/elements.py", line 58, in __init__
    self.metadata = metadata
  File "/home/caleb/projects/aur/python-panflute/src/panflute-2.0.4/panflute/elements.py", line 80, in metadata
    self._metadata = MetaMap(*value.items())
  File "/home/caleb/projects/aur/python-panflute/src/panflute-2.0.4/panflute/elements.py", line 1048, in __init__
    self._content = DictContainer(*args, oktypes=MetaValue, parent=self)
  File "/home/caleb/projects/aur/python-panflute/src/panflute-2.0.4/panflute/containers.py", line 110, in __init__
    self.update(args)  # Must be a sequence of tuples
  File "/usr/lib/python3.8/_collections_abc.py", line 838, in update
    self[key] = value
  File "/home/caleb/projects/aur/python-panflute/src/panflute-2.0.4/panflute/containers.py", line 126, in __setitem__
    v = check_type(v, self.oktypes)
  File "/home/caleb/projects/aur/python-panflute/src/panflute-2.0.4/panflute/utils.py", line 71, in check_type
    raise TypeError(msg)
TypeError: 

Element "MetaMap" received "Para" but expected <class 'panflute.base.MetaValue'>

The package itself seems to build and run okay, just the test suite is broken somehow.

@ickc
Copy link
Collaborator

ickc commented Nov 10, 2020

As shown in the README.md, panflute 2 only support pandoc 2.11.0.4 or above.

@ickc ickc closed this as completed Nov 10, 2020
@alerque
Copy link
Contributor Author

alerque commented Nov 10, 2020

Oops, sorry missed that!

@alerque
Copy link
Contributor Author

alerque commented Nov 10, 2020

I've updated to Pandoc 2.11.1.1 and am still encountering the same issue :-(

@sergiocorreia
Copy link
Owner

What's going on:

We use pytest to run our tests. pytest runs all scripts in the tests folder that start with test_. This includes test_basics.py, test_fenced.py, etc.

Instead, setup.py seems to be also executing the file tests\quick.py.

This means we have a few problems:

  1. We want to either delete quick.py if it's not relevant anymore, or fix it and rename it to test_quick.py.
  2. We need to ensure that setup.py test does the same thing that pytest does. I'm not sure how exactly setup.py test works, so I'm a bit confused

@sergiocorreia
Copy link
Owner

This should be working now, but reopened due to the question of wth is setup.py test doing and how to get it to just do what pytest does :)

Also, do you need me to bump the version and republish to pypi?

@alerque
Copy link
Contributor Author

alerque commented Nov 10, 2020

I don't think a version bump is urgent for this (for Arch) as I went ahead and pushed the package update up, but people have to build it with --nocheck for new. Given that Pandoc updates haven't landed yet there is probably a few days of leeway before it affects any but the most ambitious. I know Debian and some others are stricter about requiring tests to pass if they exist in packages. Maybe figure out for sure how setup.py tests work before bumping.

By the way the reason running tests this way and not through tox or pytest is important is that instead of testing the library (which tox and pytest are great for since they setup virtual environments with exact matches for the project dependencies) we want to test the library in the actual context of system supplied dependencies. There is not much point in a distro testing whether an upstream package passes its own regression tests in a virtual clean room (the project should be doing that anyway), but there is a lot of point in making sure it works right in the distro context with packages that may or may not be an exact version match for the dependencies and may or may not have all the right options.

@alerque
Copy link
Contributor Author

alerque commented Nov 10, 2020

I can confirm that the Arch Linux python-panflute-git package now builds properly (from Git HEAD) with checks enabled normally.

@sergiocorreia
Copy link
Owner

By the way the reason running tests this way and not through tox or pytest is important is that instead of testing the library (which tox and pytest are great for since they setup virtual environments with exact matches for the project dependencies) we want to test the library in the actual context of system supplied dependencies. There is not much point in a distro testing whether an upstream package passes its own regression tests in a virtual clean room (the project should be doing that anyway), but there is a lot of point in making sure it works right in the distro context with packages that may or may not be an exact version match for the dependencies and may or may not have all the right options.

That makes a lot of sense. I tried searching at what setup.py test did and I'm still a bit confused:

This leaves two questions:

  1. What's the plan in Arch to run tests once setup.py test is gone? (we should be targeting that IMO)
  2. Is there a way to use tox, or to adapt pytest to work without a virtualenv, so we can test that things work in the distro context, as you mention?

I'm not sure of either, so I'll leave the issue open in case someone knows the answer

@ickc
Copy link
Collaborator

ickc commented Nov 10, 2020

One option is to have a very simple test that call panflute somehow. e.g. homebrew test pandoc by converting a one-line text. The reason for test in package manager is different—they want to make sure it is installed correctly. Imagine some test like in pandoc that runs for almost an hour, a package manager wouldn't want to run the whole test suite there.

@dhimmel
Copy link
Contributor

dhimmel commented Nov 10, 2020

I just got a similar error message trying to run panflute 2.0.4 as noted at manubot/rootstock#386 (comment). Here's the CI log and error:

  File "/usr/share/miniconda3/envs/manubot/lib/python3.7/site-packages/panflute/containers.py", line 76, in insert
    v = check_type(v, self.oktypes)
  File "/usr/share/miniconda3/envs/manubot/lib/python3.7/site-packages/panflute/utils.py", line 71, in check_type
    raise TypeError(msg)
TypeError: 

Element "MetaList" received "CSL_Item" but expected <class 'panflute.base.MetaValue'>

In our case, I think we're passing a List of CSL_Item elements to the metadata. CSL_Item subclasses dict.

Error is coming from

# Invalid type
caller = get_caller_name()
tag = type(value).__name__
msg = '\n\nElement "{}" received "{}" but expected {}\n'.format(caller, tag, oktypes)
raise TypeError(msg)

Is this the same issue?

@sergiocorreia
Copy link
Owner

Sorry, just commented here: manubot/rootstock#386 (comment)

Meta elements in Pandoc are a bit restrictive in what they can accept, and AFAIK you can only assign certain elements to them (listed in pandoc-types, as mentioned in the comment).

@ickc
Copy link
Collaborator

ickc commented Dec 18, 2020

Getting back to this. I believe @dhimmel's comment has been resolved.

@alerque, is it resolved already? e.g. in another packaging effort in https://github.com/conda-forge/panflute-feedstock/blob/master/recipe/meta.yaml#L35-L45 , we just test packaging error by import panflute, pip check, and pandoc -F panflute.

pytest cannot be run there because of a potential packaging issue, that tests are not included in sdist. But that's a separate issue.

Closing?

@alerque
Copy link
Contributor Author

alerque commented Dec 19, 2020

Yes I just checked again and I think we're good here as far as Arch Linux packaging is concerned.

@ickc
Copy link
Collaborator

ickc commented Dec 19, 2020

Great!

@ickc ickc closed this as completed Dec 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants