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

[tests-only] Explicitly use python2 for carddav and caldav tests #40054

Merged
merged 1 commit into from
May 25, 2022

Conversation

phil-davis
Copy link
Contributor

@phil-davis phil-davis commented May 6, 2022

Description

The owncloud-ci images for Ubuntu 18.04 currently have python being python v2, and python3 runs python v3. But with Ubuntu 20.04 (and later...) that may or may not be the case - python might run python v3, or the python command might not exist at all or...

The CalDAV and CardDAV test suites use python and require python v2. They are from GitHub repos like https://github.com/apple/ccs-caldavtester that have not been maintained for years, and so do not support python v3.

I am currently looking at things that fail in CI with Ubuntu 20.04 - issue #38348 - and this is one thing.

All the Ubuntu-based images that we use (should) have a valid python2. So use that in the existing CI, and it will "just work" when we want to move to Ubuntu 20.04 and later.

This should also help the (rare) developer who wants to run these scripts locally - they won't have to worry about which version their python command is.

Related Issue

Part of #38348

How Has This Been Tested?

CI

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@phil-davis phil-davis self-assigned this May 6, 2022
@phil-davis phil-davis requested review from xoxys and individual-it May 6, 2022 08:00
@phil-davis phil-davis marked this pull request as ready for review May 6, 2022 08:00
@xoxys
Copy link
Contributor

xoxys commented May 6, 2022

If we want to continue using this tool we should maintain it on our own. Python2 should not be used anymore.

@phil-davis
Copy link
Contributor Author

If we want to continue using this tool we should maintain it on our own. Python2 should not be used anymore.

what to do right now?

I can add "2" here to "python" now to help for some existing and upcoming Ubuntu releases, or block waiting for "someone" to fork the cardav/caldav test repos and make them run with python3. Who will "someone" be, and what time resources will they be given.

@phil-davis
Copy link
Contributor Author

what to do right now?

I can add "2" here to "python" now to help for some existing and upcoming Ubuntu releases, or block waiting for "someone" to fork the cardav/caldav test repos and make them run with python3. Who will "someone" be, and what time resources will they be given.

@micbar I am not sure who to ask about this. To get tests running on Ubuntu 20.04 (and then Ubuntu 22.04) we will need some way to run these caldav/carddav tests. The 2 solutions that I can think are are:

  1. this PR - explicitly specify python2 (which is what we are actually running in CI currently on Ubuntu 18.04), or:
  2. fork the caldav/carddav test repos and work on the code so that it runs with python3 (takes some unknown effort)

what to do?

@xoxys
Copy link
Contributor

xoxys commented May 10, 2022

I can offer a fork and do minimal maintenance to support python 3 in two weeks. Shouldnt be that much effort.

@phil-davis phil-davis force-pushed the python2-caldav-carddav branch from c70669f to 835cea9 Compare May 16, 2022 04:22
@phil-davis
Copy link
Contributor Author

phil-davis commented May 16, 2022

Changed to "Blocked" column, set to draft. Needs python test code that will run with python3.

@phil-davis phil-davis marked this pull request as draft May 16, 2022 12:14
@phil-davis
Copy link
Contributor Author

phil-davis commented May 24, 2022

To run a set of tests locally, I reverse-engineered what drone CI does:

  • have an oC10 server installed locally
  • from the root folder of the install (e.g. mine is /home/phil/git/owncloud/core ) clone the needed repos, create user01 and user02 and the expected calendars:
cd apps/dav/tests/ci/caldav
git clone https://github.com/apple/ccs-caldavtester.git CalDAVTester
git clone https://github.com/apple/ccs-pycalendar.git pycalendar
cd ../../../../../
OC_PASS=user01 php occ user:add --password-from-env user01
php occ dav:create-calendar user01 calendar
php occ dav:create-calendar user01 shared
OC_PASS=user02 php occ user:add --password-from-env user02
php occ dav:create-calendar user02 calendar
  • run the PHP dev server in a terminal window of its own:
php -S 127.0.0.1:8889
  • run the test suite:
cd apps/dav/tests/ci/caldav/CalDAVTester
PYTHONPATH="./pycalendar/src" python testcaldav.py --print-details-onfail --basedir "../../caldavtest/" -o cdt.txt \
	"CalDAV/current-user-principal.xml" \
	"CalDAV/sync-report.xml" \
	"CalDAV/sharing-calendars.xml"

I got mostly passes, 4 fails, so there is something in my local installation that is not quite the same as in CI (which is set up completely from scratch)

But this should be enough to help in doing a python3 refactoring - when I specify python2 it runs, when I specify python3 it falls over with:

Traceback (most recent call last):
  File "testcaldav.py", line 22, in <module>
    from src.manager import manager
  File "/home/phil/git/owncloud/core/apps/dav/tests/ci/caldav/CalDAVTester/src/manager.py", line 80
    print str
          ^
SyntaxError: Missing parentheses in call to 'print'. Did you mean print(str)?

That will be the first of perhaps many things that need to be changed.

Note: there are 4 test suites:
apps/dav/tests/ci/caldav
apps/dav/tests/ci/caldav-old-endpoint
apps/dav/tests/ci/carddav
apps/dav/tests/ci/carddav-old-endpoint

But hopefully the changes needed in the cloned test repos for python3 will become obvious, and the test code can have the changes applied, then it will all "magically" pass in drone CI (without bothering to run every test suite locally)

@xoxys
Copy link
Contributor

xoxys commented May 25, 2022

@phil-davis Even if it runs now with python3 without a python error, everything else explodes.... The entire codebase is pretty fragile... not sure if we should go this route, especially as I expect more effort now.

@phil-davis
Copy link
Contributor Author

@xoxys then from my PoV we can specify python2 for now - that keeps this thing working on newer Ubuntu that defaults python to python3.

And I can create a separate issue about this, and the development and QA team can discuss what to do with this special test suite. If we keep it for "the medium term" or longer, then "someone" will need to allocate and schedule effort to update it...

@xoxys
Copy link
Contributor

xoxys commented May 25, 2022

Alright, let's do it this way, I'll revert my commit to restore your state.

@phil-davis phil-davis force-pushed the python2-caldav-carddav branch from f815d03 to acc68d7 Compare May 25, 2022 11:58
@phil-davis phil-davis marked this pull request as ready for review May 25, 2022 11:59
@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline apiWebdavMove1-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/35865/94

@phil-davis
Copy link
Contributor Author

Alright, let's do it this way, I'll revert my commit to restore your state.

And I have rebased and squashed back to the original change.

@xoxys @individual-it please review again.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@phil-davis phil-davis merged commit 1252e5d into master May 25, 2022
@delete-merged-branch delete-merged-branch bot deleted the python2-caldav-carddav branch May 25, 2022 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants