search

OpenTreeMap / otm-core

OpenTreeMap is a collaborative platform for crowdsourced tree inventory, ecosystem services calculations, urban forestry analysis, and community engagement.
www.opentreemap.org
Other
186 stars 88 forks source link

First pass at fixing tests from Py2->3 upgrade #3288

Closed fungjj92 closed 4 years ago

fungjj92 commented 4 years ago

Overview

This PR is a first go at fixing the failing tests from the Py2->3 upgrade. The tests should be fixed for the exporter app 🎉 One down.

I touched tests / code across the django apps api, exporter, importer, treemap. Except for exporter, they continue to have errors. I'm not sure how the test runner selects the order of tests to run because it skipped around these 4 apps when running ./scripts/manage.py test --failfast

Most of the changes thus far have been about unicode/byte - string translation. The celery and Django-tinsel upgrades were to fix test-related errors, see commit https://github.com/OpenTreeMap/otm-core/commit/df782b1e40ab988e27bede383616c0587344a2c2

Notes

In retrospect, I would have worked on one app's tests at a time. My advice to parallelize this work moving forward is just that: ./scripts/manage.py test <app_name> --failfast

It's impossible to linearly compare progress for various reasons. But anyway, we're making progress!

Before:

Ran 1115 tests in 275.613s

FAILED (failures=19, errors=305, skipped=23)

And this is what remains after related-PRs 😳 

Ran 1115 tests in 331.760s

FAILED (failures=26, errors=125, skipped=23)

Testing

In the app VM, pip install the new python dependencies: vagrant ssh app -c 'cd /opt/app/core && envdir /etc/otm.d/env pip3 install -r ./requirements.txt'

Checkout the otm-addons companion branch https://github.com/OpenTreeMap/otm-addons/pull/1566

I suppose you'll want to test each of the apps that was touched and check if any still failling tests are a result of lines of codes touched in this PR. ./scripts/manage.py test <app_name>

Also ensure that the changes don't chip away important context just to get tests to work.

fungjj92 commented 4 years ago

Standby. I am in the middle of fixing auth test issues in this branch, I realized there were some errors I was making in encoding/decoding.

fungjj92 commented 4 years ago

Alright this is ready to be looked at.

Ran 1115 tests in 297.171s

FAILED (failures=25, errors=125, skipped=23)

I had been clobbering the basic auth tests with unwanted layers of encoding & decoding. I'm not sure how these failures didn't arise earlier, but alas. I was tipped off by the lack of output at the regex parsers; no surprise but regex is great at preserving and communicating its desired data format, which was helpful in figuring out how the auth data should look.

jwalgran commented 4 years ago

I did provision a fresh app VM and got the same results

FAILED (failures=25, errors=264, skipped=23)
fungjj92 commented 4 years ago

Thank you for the well-considered rounds of reviews. Onward! Merging.