2.x release doesn't pass tests

[alerque]

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]

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

[alerque]

Oops, sorry missed that!

[alerque]

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

[sergiocorreia]

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]

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]

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]

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

[sergiocorreia]

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:

• It seems that no tests are being run when I execute setup.py test now, which sure doesn't raise an error but doesn't test that the tests pass either.
• https://setuptools.readthedocs.io/en/latest/userguide/commands.html?highlight=test#test-build-package-and-run-a-unittest-suite warns that "test 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." It also states that it requires a test_suite argument either in the command line or in setup(), and we have neither, so no idea how it decided to run the file in tests.
• Deprecate test command pypa/setuptools#1684 also mentions the deprecated command

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]

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]

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

panflute/panflute/utils.py

Lines 67 to 71 in 32fc99b

	# 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]

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]

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]

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

[ickc]

Great!