Skip to content

[v7r2] Run pytest test collection for Python 3#4718

Closed
chrisburr wants to merge 20 commits into
DIRACGrid:integrationfrom
chrisburr:py3-pytest-discover
Closed

[v7r2] Run pytest test collection for Python 3#4718
chrisburr wants to merge 20 commits into
DIRACGrid:integrationfrom
chrisburr:py3-pytest-discover

Conversation

@chrisburr

Copy link
Copy Markdown
Member

This PR adds pytest --collect-only to the CI for Python 3 and none of the changes should make a difference in Python 2. More changes will be needed before the tests work but this will at least ensure that it's possible to import most of DIRAC. All of the changes should be transparent in Python 2 and future PRs can then start enabling parts of the unit tests by adding -k mytests to the command.

I've constructed it to be easy to review as individual commits instead of in one go. I've split them into two groups:

Simple renaming of modules that is handled using six

  • 62b10d7: Replace "import thread" with "from six.moves import _thread as thread"
  • b9b6299: Replace "import urlparse" with "import six.moves.urllib.parse"
  • 4bdc9b0: Replace "import Queue" with "from six.moves import queue as Queue"
  • 331c7ed: Replace "itertools.izip_longest" with "six.moves.zip_longest"
  • 58d240d: Get urllib2.URLError from six.moves.urllib_error
  • 7ece6c3: Replace StringIO with six.StringIO and six.BytesIO
    • In Python 2 six.StringIO and six.BytesIO are both StringIO.StringIO. In Python 3 they are io.StringIO and io.BytesIO but it's not practical to use these in Python 2 as StringIO requires unicode objects as input which isn't worth it. I've tried to guess at which one will be appropriate for Python 3 but more changes will definitely be needed here to make the tests pass once they're being executed.

Other changes

  • 8a4eaf1: Fallback to subprocess.getstatusoutput for commands.getstatusoutput
    • These should be rewritten to use subprocess32 properly rather than using the long-deprecated getstatusoutput. (the subprocess32 doesn't include Python 3's subprocess.getstatusoutput)
  • d285b56: Encode strings as UTF-8 before using hashlib.md5
  • 604de9c: Allow importing DEncode in Python 3 without actually porting it
  • 0716316: Minimal types fixes to make pytest discovery functional
  • 69f058b: Fix string type checking in Core/Utilities/Time.py
  • 846d8a6: Escape backslash in Core/Utilities/Grid.py
  • 4f3f839: Remove Python version check on import
    • DIRAC errors on import for anything except Python 2.7 but I don't see any reason to keep this check given dirac-install will force people to get Python 2.7 from DIRACOS.
  • 5a66509: Remove MySQLdb.server_init if using mysqlclient
    • MySQL-python was abandoned a long time ago and never supported Python 3. It looks like mysqlclient is almost a drop in replacement so that's what I've gone with for now. The only change it seems to need is this commit.
  • c8c1f9d: Run pytest discovery with Python 3
    • I add a new environment-py3.yml file to manage the Python 3 dependencies. It's ugly to list them in yet another place but I think it's necessary given things won't be exactly the same.
    • fts-rest Doesn't seem to support Python 3 at all. I've made enough changes in a fork to make it possible import and that's what I use here. Porting it properly to using six should only take a few hours but I didn't find any tests and we should ask the developers to see what their plans are.

BEGINRELEASENOTES

*Python 3
NEW: Run pytest test collection for Python 3

ENDRELEASENOTES

import time
import select
import cStringIO
from six import BytesIO

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I didn't check, but when StringIO or BytesIO`?

@chrisburr chrisburr Aug 17, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's analogous to opening a file with open(filename, mode="wt") and open(filename, mode="wb") though Python 2 doesn't do a very good job teaching you to think about the difference.

  • StringIO should be used when you want something you would open in a text editor: XML files, JDL, HTML.
  • BytesIO should be used when you just want a bag of bytes such as a compressed tarfile or an image. Also if you want to hash something with md5, you need bytes as different encodings will result in different hashes.

I think I've got these correct but we'll only know for sure when we start executing tests. I have a draft branch locally which gets 80% of them passing so I'll make a PR to start executing tests as soon as this PR is merged.

Comment thread Core/scripts/dirac-externals-requirements.py Outdated
Comment thread Resources/ProxyProvider/test/Test_DIRACCAProxyProvider.py Outdated
Comment thread tests/Integration/Framework/Test_ProxyDB.py Outdated
Co-authored-by: fstagni <federico.stagni@cern.ch>
Comment thread Core/Utilities/test/Test_Encode.py Outdated
@chaen

chaen commented Aug 26, 2020

Copy link
Copy Markdown
Contributor

For fts-rest I know it was on their roadmap, I'll check with them

@chaen

chaen commented Aug 26, 2020

Copy link
Copy Markdown
Contributor

Argh !! When commenting a specific commit, it does not display it nicely.... 😢

@chrisburr

Copy link
Copy Markdown
Member Author

Argh !! When commenting a specific commit, it does not display it nicely.... 😢

It's not so bad. You can click the filename in the heading bar to get to the conversation view which I think works better than a huge "changes" view on the PR tab.

@chaen

chaen commented Aug 26, 2020

Copy link
Copy Markdown
Contributor

It's not so bad. You can click the filename in the heading bar to get to the conversation view which I think works better than a huge "changes" view on the PR tab.

Somehow on the other PR it worked, while I don't think I've done anything different... weird

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.

5 participants