dsnopek / anki-sync-server

A personal Anki sync server (so you can sync against your own server rather than AnkiWeb)
GNU Affero General Public License v3.0
747 stars 95 forks source link

Add helper classes for integration tests, refactor user managers and ankiserverctl.py #35

Open cdpm opened 8 years ago

cdpm commented 8 years ago

Add test helper classes for creating and managing temporary files, working with anki collections and sqlite dbs. Add class for managing users so users can be added programmatically without using ankiserverctl.

dsnopek commented 8 years ago

This doesn't change any existing code to use these classes. Is that intentional (ie. these are meant to be completely seperate)? Or is that coming as a next step?

It's definitely easier to review if they are meant to be completely seperate, because it doesn't change anything, I can just merge it. :-) But if later code is going to be changed to use these classes, I'd prefer to review the addition of the new classes at the same time as the changes to the existing code.

Thanks!

cdpm commented 8 years ago

I wrote some integration tests for SyncApp using WebTest that depend on the new test helpers but don't change existing code. I'll hopefully be able to review and commit them in this pull request in about a week.

cdpm commented 8 years ago

Made a second commit a few weeks ago with the integration test code. In the future It might be useful to refactor ankiserverctl to use UserManager to avoid duplicate logic for managing users, but there are no direct changes to existing code right now.

dsnopek commented 8 years ago

Thanks! I still think it'd be nice to have the changes to the existing code in the same PR, because it would be easier for me to review.

cdpm commented 8 years ago

Committed changes to existing code.

dsnopek commented 8 years ago

I've added a Travis CI configuration to the master branch. Please merge from master into your PR, and make any changes necessary to get the tests passing on Travis. Thanks!

dsnopek commented 8 years ago

Thanks! The tests are failing because the 'sqldiff' command is unavailable, but the tests that don't depend on that appear to be working