Test suite showing some ageing cracks

[alerque]

I'm the packager for this on Arch Linux and I'm starting to run into some problems. I realize you use Tox here for testing and that makes sense for your upstream use case, but it does not allow testing in place on a system to confirm that distro packages are serving their functions.

First, using setuptools as a test runner stopped working a while back because datetime as used in this project is no longer compatible with the current released version of datetime.

Second, switching to pytest as a runner, I get the following test failure. I presume this is again a time parsing issue related to Python upstream datetime changes.

============================= test session starts ==============================
platform linux -- Python 3.9.6, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
rootdir: /build/python-agate-sql/src/agate-sql-0.5.7
collected 18 items

tests/test_agatesql.py ...............F..                                [100%]

=================================== FAILURES ===================================
_______________ TestSQL.test_to_sql_create_statement_with_schema _______________

self = <tests.test_agatesql.TestSQL testMethod=test_to_sql_create_statement_with_schema>

        def test_to_sql_create_statement_with_schema(self):
            statement = self.table.to_sql_create_statement('test_table', db_schema='test_schema', dialect='mysql')

>           self.assertEqual(statement.replace('\t', '  '), '''CREATE TABLE test_schema.test_table (
      number DECIMAL(38, 3),
      textcol VARCHAR(1) NOT NULL,
      boolean BOOL,
      date DATE,
      datetime TIMESTAMP NULL
    );''')  # noqa
E   AssertionError: 'CREA[126 chars]\n  datetime TIMESTAMP NULL, \n  CHECK (boolean IN (0, 1))\n);' != 'CREA[126 chars]\n  datetime TIMESTAMP NULL\n);'
E     CREATE TABLE test_schema.test_table (
E       number DECIMAL(38, 3),
E       textcol VARCHAR(1) NOT NULL,
E       boolean BOOL,
E       date DATE,
E   -   datetime TIMESTAMP NULL,
E   ?                          --
E   +   datetime TIMESTAMP NULL
E   -   CHECK (boolean IN (0, 1))
E     );

tests/test_agatesql.py:142: AssertionError
=============================== warnings summary ===============================
../../../../usr/lib/python3.9/site-packages/leather/series/base.py:3
../../../../usr/lib/python3.9/site-packages/leather/series/base.py:3
../../../../usr/lib/python3.9/site-packages/leather/series/base.py:3
  /usr/lib/python3.9/site-packages/leather/series/base.py:3: DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated since Python 3.3, and in 3.10 it will stop working

tests/test_agatesql.py: 400 warnings
  /usr/lib/python3.9/site-packages/packaging/version.py:127: DeprecationWarning: Creating a LegacyVersion has been deprecated and will be removed in the next major release

-- Docs: https://docs.pytest.org/en/stable/warnings.html
=========================== short test summary info ============================
FAILED tests/test_agatesql.py::TestSQL::test_to_sql_create_statement_with_schema
================== 1 failed, 17 passed, 403 warnings in 0.62s ==================

Additionally it's worth noting that some other deprecations warnings are in play and Python 3.10 is going to add another failure, although it looks like that might be in a transitive dependency.

It is probably worth updating this project to use current APIs and doing a minor release so that it can be used more robustly than inside a private venv bubble with old releases of stuff.

[jpmckinney]

The current release doesn't use tox. It runs tests with nose:

pip install .[test]
nosetests

You can see all tests green here: https://github.com/wireservice/agate-sql/actions/runs/1034818027

Whether CHECK (boolean IN (0, 1)) appears (as in your quoted failure) depends on the versions of SQLAlchemy and maybe the MySQL adapter being used. You can compare against the test environments in the above build.

[alerque]

The difference between pytest and nose is irrelevant here. I get the same error when testing with nosetests as the test runner.

I'm pretty sure the issue is going to turn out to be that your tests require old versions of dependencies. Note from your passing test log:

 Collecting parsedatetime!=2.5,!=2.6,>=2.1
   Downloading parsedatetime-2.4.tar.gz (58 kB)

I'm testing on a distro that has has up to date versions of all dependencies. Just because your version of testing with pinned versions of everything pass doesn't mean all is well. That will help people that run venv's and know the right things to hold back. However an argument can be made that if it doesn't work on current stable released versions of dependencies that is at least an aging crack if not a bug. In this case the actual library seems to work fine still, just the tests fail, but being able to run tests is very valuable for distro packages that need to check if everything is found in the right places etc.

[alerque]

Just for background, this is issue is being raised at all because I'm trying to get csvkit and hence its dependencies into the default official Arch Linux package repositories. Vendoring dependencies is not an option.

[jpmckinney]

parsedatetime introduced a breaking change. That is why we exclude specific versions. See:

https://github.com/certbot/certbot/issues/8042 https://github.com/bear/parsedatetime/issues/246 https://github.com/pyca/cryptography/pull/5264 https://github.com/dssg/triage/issues/721 https://github.com/wireservice/agate/issues/743 https://github.com/wireservice/csvkit/issues/1081

We are not the only project to reject those versions of parsedatetime. If, for Arch Linux, it is necessary to use the latest version of parsedatetime, even if it breaks lots of things like certbot, then I guess we will just have to hope that parsedatetime's maintainers un-break things.

[jpmckinney]

OpenSUSE seems to have written a patch https://github.com/bmwiedemann/openSUSE/commit/6ba73c5eb4f4e53416620de528c25e3138d44993 to revert the change in https://github.com/wireservice/agate-sql/issues/33

As I mention in #33, the issue seems to be that, depending on the version of SQLAlchemy and MySQL, either the CHECK (boolean IN (0, 1)) constraint gets added, or it doesn't. But the existence of this constraint has no impact on csvkit or agate-sql whatsoever. So, I can just as well relax the test.

[jpmckinney]

The tests should pass now!

[alerque]

Great, thanks! Will there be a release soon to reflect this? Or should I try applying that SUSE patch for now? I'll hold off a few days if a release is immanent.

[jpmckinney]

0.5.8 should appear on PyPi shortly 🚀