Python 3.12 tests currently fail

[facelessuser]

The error is:

Traceback (most recent call last):
  File "/Users/facelessuser/Code/github/markdown/tests/test_meta.py", line 20, in test__version__IsValid
    import packaging.version
ModuleNotFoundError: No module named 'packaging'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/facelessuser/Code/github/markdown/tests/test_meta.py", line 22, in test__version__IsValid
    from pkg_resources.extern import packaging
ModuleNotFoundError: No module named 'pkg_resources'

This will require more investigation. 3.12 is still in beta. We will need to be fixed before we officially state support for 3.12.

[waylan]

@pawamoy stated in this comment:

https://docs.python.org/3.12/whatsnew/3.12.html#removed:

easy_install, pkg_resources, setuptools and distutils are no longer provided by default in environments created with venv or bootstrapped with ensurepip, since they are part of the setuptools package. For projects relying on these at runtime, the setuptools project should be declared as a dependency and installed separately (typically, using pip).

It seems packaging should be added as a test dependency. As a fallback maybe we can use importlib.resources instead of pkg_resource.

[waylan]

It seems packaging should be added as a test dependency. As a fallback maybe we can use importlib.resources instead of pkg_resource.

Note that the error results from from pkg_resources.extern import packaging. We are using packaging for version parsing, which is not part of importlib and still requires packaging. IIRC, we were intentionally importing packaging this way to ensure that the same version of the lib was used that was used for installation. However, now that pkg_resources is deprecated (even by Setuptools) and importlib does not seem to have any extern.packaging (or similar) object, we should just import packaging directly. Of course, that would require that packaging be included in the list of dependencies for our test envs as well.

[waylan]

Did some digging through the history and found this comment which explains why we did not need to list packaging as a dependency (it also alludes to why we used pkg_resources.extern.packaging). Specifically, having pkg_resources installed guaranteed that packaging would be installed. Oh how things change.

[facelessuser]

While I understand why we rely on packaging, it'd be nice to just eliminate such a dependency with a simplistic approach, but I realize this may not happen.

[waylan]

it'd be nice to just eliminate such a dependency with a simplistic approach

I was thinking similarly. In the relevant tests, we are comparing the output of markdown.__meta__._get_version to parsed version strings, where the parsing is done by packaging. Why not just compare the output to the string directly?

According to the commit message (in 82d2b25) the thinking was that by parsing the string, we were ensuring that we were outputting a normalized format. But what is going to cause those tests to fail in the future? It's not likely that packaging will change its behavior for the current API. I would expect a new API would be introduced for a new format. So, I suppose, in the end there isn't much value in keeping the tests as-is.

I could go either way on this. In the one hand, it isn't much work to add one more dependency to the existing list. On the other had, if the tests are more complex than they need to be, why not simplify them?

[waylan]

Just noting that #760 tracks the Python versions we support. Python 3.7 reached end-of life on 2023-06-27 and Python 3.12 is scheduled for release on 2023-10-02. Therefore, I think it makes sense to drop support for 3.7 at the same time that we add official support for 3.12 (even if we do so prior to 31.2's final release).

As an aside, I will note that this will be the last time that Python drops a version on a different date that it releases a new version. Going forward, it is expected that both are to always happen in October of each year. Then we shouldn't need to be concerned with addressing them separately any more. While we could this year, I think we may as well combine them now anyway.

Finally, I am assigning this to the 3.5 milestone. This needs to happen in the next point release we make. Hopefully we can get it done by 2023-10-02 or shortly thereafter.

[waylan]

In the relevant tests, we are comparing the output of markdown.__meta__._get_version to parsed version strings, where the parsing is done by packaging. Why not just compare the output to the string directly?

So we are already just comparing the string. The one test where we call packaging is for the current version. It is ensuring the the actual version number is the normalized form. As it is generated (and changes with each release) there is no way to know what the expected string is ahead of time without a manual edit. In other words, we either need to drop the test__version__IsValid test or list packaging as a dependency.

[facelessuser]

I personally opt for just dropping the test. Unless you are truly concerned we are going to have an issue with people putting in invalid versions that we won't catch in code review. I'd like to avoid dependencies for something like this, but that is my opinion.

[mitya57]

Another option would be to raise unittest.SkipTest if packaging module is not available.

This way we won't need it most of the time, but if you bumped a version and want to double-check its correctness, you can run tests with packaging installed.

[waylan]

I was just thinking similarly. The question is, how do we want to determine when to install the extra dependency? In practice, we only need the test to run when we edit the version. I see a few options:

1. Check for edits to the markdown/__meta__.py file.
2. Check the Commit message for "Bump version to...".
3. Check that a label has been applied to the PR.

Option 1 is the easiest way to not miss anything. Option 2 assumes the documented release process is being followed. And Option 3 would require an extra step in the release process. I'm inclined to go with option 1 as it is less prone to human error. However, with option 3 it is always possible to add the label later and rerun the tests. Option 2 seems like it is the most fragile as a simple typo could easily cause it to get missed.

I think I will attempt option 1 and if it proves too difficult to get a GitHub workflow to properly detect the edited file, then I will switch to option 3. Unless someone has a better suggestion...

[waylan]

That was easier than expected. See a working solution in #1371. Thanks @mitya57 for the suggestion.

[facelessuser]

I like this approach