internetarchive/infogami

Ensure current unit tests pass on CI under Python 2.7

Closed this issue · 6 comments

hornc commented

Commit b8831c7 removed the existing tests from running on CI, which enabled a parse error to slip into master, involving a try with a missing except block which is fixed by #20

After some discussion (amongst archive.org staff and volunteers) we agreed the following:

  • We need this repo for the continued functioning of the Open Library project https://github.com/internetarchive/openlibrary
  • We have committed to making the Open Library codebase Python 3 compatible due to the impending Python 2 EOL

Therefore we need to upgrade this internetarchive/infogami fork to Python 3 to support Open Library.

To do this safely since this codebase has had had much activity for a long time, we need to have confidence that we are not breaking anything in our fork with current changes.

  • Get existing unit tests running and passing on CI under Python2.7 to provide a baseline for our fork (run under Python 3 for information only, in allowed failures)
  • Get flake8 tests passing under Python 2.7
  • Focus on fixing Python 3 failures, move this out of allowed failures
cclauss commented

My apologies for b8831c7 and thanks for you efforts to repair it. The plan above sounds most resonable to me.

hornc commented

I have a branch https://github.com/internetarchive/infogami/tree/repair-tests where the Travis CI results match the test results I get locally at this commit
85ff436

Results: https://travis-ci.org/internetarchive/infogami/jobs/573626882

Which is failing on the issue reported at infogami#4

I thought I'd fixed it in #9 , but the 'unused' import I removed is used in that file, so it's not a good fix.

To add the suggested app = web.application(urls, globals(), autoreload=False) fix, we need to provide something sensible for urls in delegate.py, I'm not immediately sure what that should be.

cclauss commented

For now, can we put a # noqa linter directive on the import line so that we can progress beyond that one?

hornc commented

The app issue that caused tests to fail was resolved in 7ce3879 by changing the order of importing delegate

cclauss commented

Can this be closed now?

hornc commented

yes, Closing as done, Python 2.7 tests are passing.

  • 👍1