search

emmett-framework / emmett

The web framework for inventors
BSD 3-Clause "New" or "Revised" License
1.06k stars 71 forks source link

:white_check_mark: Add integration test #162

Closed mijdavis2 closed 7 years ago

mijdavis2 commented 7 years ago

Problem

No integration tests. This resulted in an AuthUser bug in 0.7.9 that wasn't caught for too long.

Solution

Result

🎉 more test coverage 😄

Caveats

This is a bit beefy for the integration coverage this gets us.

Not sure if that's fine and we can just add on tests to this test app or if it's preferable to strip down the app to bare minimum to get the test coverage. Maybe even better to pull out test app into a submodule? Thoughts?

mijdavis2 commented 7 years ago

WIP - need to fix ci testing config

gi0baro commented 7 years ago

@mijdavis2 wow, this is huge.

I really appreciate your efforts on this, but I think we can build up something light for the auth integration tests, maybe using the weppy included testing client.

Moreover, in the next weeks/months I will proceed with #134, so the auth module will be something totally different and writing tests during the refactoring seems to be smarter, since we can avoid to rewrite them again after the refactoring.

Maybe I can assign something to you during the refactoring, but right now I think the best option is to reject this PR :/

Let me know if you have any considerations about this.

mijdavis2 commented 7 years ago

@gi0baro - hey no worries!

It is basically a copy/paste from additions I've made to starter_weppy so no big deal. This forced me to get acquainted with tox and phantomjs so still worth it 👍 .