AuburnACM / auacm

The Auburn ACM Website
Apache License 2.0
15 stars 3 forks source link

Continuous Integegration #87

Open BrandonLMorris opened 8 years ago

BrandonLMorris commented 8 years ago

This one is going to be a doozie.

Using a tool like Travis CI, every push (at least to master) should trigger an automatic build and test run. Any errors should be caught by this system and addressed before deploying to production. Additionally we can look into having continuous deployment and having the production server pull the passed code automatically.

However, this is going to require a lot of test coverage and automation:

The benefits would be enormous, allowing for simplified deployment (both for development and production) and better assurance in the code.

WilliamHester commented 7 years ago

After looking through the AUACM (server) code more, it becomes quite apparent that we didn’t design with testing in-mind. There’s a lot of work to be done here.

A large part of the work is on the fact that the database queries are hard to mock out (and really should be mocked out), so I might focus on mocking those calls. Having a test database seems okay at best; I think we should really be unit-testing rather than depending upon having access to the database. Ideally, the unit tests should work without having a sql server running on the computer at all (I believe this would be a requirement for CI).

One thing that will come from this is a bit of style that we use at Google. Rather than importing the class from the module, you simply import the module. This means that when the module is mocked out, the changes will propagate to the classes that depend upon that module.

WilliamHester commented 7 years ago

Another option, rather than mocking out Session, would be to instead move the logic of querying the database to another module within the different manager modules, then we could mock out that function for the unit tests.

DaveShuckerow commented 7 years ago

My recommendation would be to refactor the database access out into a service that's responsible for providing model objects to client code rather than interacting directly with the database session. You can then provide that layer to client code as a dependency, and you can mock it out with an interface that you get to define instead of having to conform to the Session object's interface, which could change over the lifetime of the database library.

El mar., 23 de ago. de 2016 22:47, William Hester notifications@github.com escribió:

Another option, rather than mocking out Session, would be to instead move the logic of querying the database to another module within the different manager modules, then we could mock out that function for the unit tests.

— You are receiving this because you are subscribed to this thread.

Reply to this email directly, view it on GitHub https://github.com/AuburnACM/auacm/issues/87#issuecomment-241963791, or mute the thread https://github.com/notifications/unsubscribe-auth/AOQUJrhbvwFB3lXbVTXt8ry7emMFURrFks5qi9sHgaJpZM4IDqhO .

WilliamHester commented 7 years ago

I agree with @DaveShuckerow here. Mocking out the database calls would be a massive undertaking, whereas moving the database-interaction code to a separate module would both make our code more modular (so we can feel trendy), and more testable. Both are huge tasks, but this could actually be a really good chance to clean up our codebase. There's a lot of repeated code (see: getting a problem out of the db). This is a good chance to centralize that and make it uniform.

BrandonLMorris commented 7 years ago

I'll have to give this some serious thought, thanks for the feedback. I guess we initially didn't create a database service because (other than naivete) SQLAlchemy's ORM kind of functions like one. But at this point, it probably is necessary to consolidate that logic into a dependable service.

I think what you're also hinting at, @WilliamHester, is that we need unit tests and real integration tests. Unit tests wouldn't require a running database, they would just ensure that the individual API methods function as we expect them to (mocking out as much as possible). The integration tests would actually run the code in a faux production environment (maybe w/ Docker?) and test drive the features (creating/deleting users, setting up a competition, etc.). Right now we have a Frankenstienian amalgamation of the two.

Does that sound about right?

WilliamHester commented 7 years ago

@BrandonLMorris That sounds right!

I would like to start here with the unit test bit. This would mean that we could slowly, one module at a time convert each module to having a separate service for querying the database, and we could create true unit tests for the endpoints.

BrandonLMorris commented 7 years ago

Sounds great. Would you mind looking into pytest? I'm seriously considering switching to it; it seems a bit more straightforward and flexible than the built-in unittest. Especially if we're going to redo a lot of our testing anyway...

WilliamHester commented 7 years ago

I'll check it out and see what it's all about.