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

Add tox for unit tests #1118

Merged

Conversation

keitherskine
Copy link
Collaborator

This PR is the first in a series of PRs in an attempt to update the way the unit tests are run. This PR adds the ability to run the unit tests through tox. Tox will automatically create a virtual environment within the top-level .tox directory, and run the tests from within that venv, which is generally more convenient than creating venvs ourselves. I have tried to make as few changes as possible to the unit test scripts for this, whilst maintaining backward-compatibility with the way the unit tests are currently run in the CICD pipelines. Consequently, no changes have needed to be made to the CICD pipelines.

Tox will run the unit tests under whatever the default Python version is currently available. I have deliberately not made tox run the unit tests under multiple versions of Python. Right now, I think the use case is for developers to be able to run the unit tests quickly and easily, and one version of Python should suffice for that. The full range of Python versions will still be tested in CICD as usual. But if people feel strongly on this, I'm happy to revisit in subsequent PRs.

You'll notice I have not tried to support Python 2.7 in tox. We should be moving away from Python 2.7, and 2.7 will still be tested in CICD anyway.

Whilst here, I have updated tests3/testutils.py to not use distutils.util.get_platform() anymore because it was deprecated a long time ago and is about to be dropped.

Also, a minor couple of additions to the .pyi file.

Copy link
Collaborator

@gordthompson gordthompson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Keith! One very minor improvement(?). Currently when one or more tests fail, tox exits with

FAILED (failures=1)
__________ summary __________
  unit_tests: commands succeeded
  congratulations :)

which might be misleading to some. This patch

Index: tests3/run_tests.py
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/tests3/run_tests.py b/tests3/run_tests.py
--- a/tests3/run_tests.py	(revision 99f27809b685d7492b3a2985e695593e2797a565)
+++ b/tests3/run_tests.py	(date 1667747041766)
@@ -1,5 +1,6 @@
 #!/usr/bin/python
 import os
+import sys
 
 import testutils
 
@@ -72,4 +73,5 @@
     import pyodbc
 
     # run the tests
-    main(**vars(args))
+    passed = main(**vars(args))
+    sys.exit(0 if passed else 1)

gets tox to exit with an error instead of "congratulations :)".

FAILED (failures=1)
ERROR: InvocationError for command /home/gord/git/pyodbc/.tox/unit_tests/bin/python ./tests3/run_tests.py --sqlserver 'DSN=mssql_199;UID=scott;PWD=tiger^5HHH' -v (exited with code 1)
___________________________________________________________________________________________ summary ____________________________________________________________________________________________
ERROR:   unit_tests: commands failed

@keitherskine
Copy link
Collaborator Author

Great point, Gord. I have updated run_tests().

Ref: https://tox.wiki/en/latest/index.html#system-overview, see point 3.3.

@gordthompson gordthompson self-requested a review November 6, 2022 17:15
Copy link
Collaborator

@gordthompson gordthompson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@keitherskine keitherskine merged commit 62b07fa into mkleehammer:master Nov 9, 2022
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.

None yet

2 participants