search

avast / pytest-docker

Docker-based integration tests
MIT License
423 stars 71 forks source link

Getting rid of tests that only test the implementation, and the path of future updates? #24

Closed butla closed 5 years ago

butla commented 5 years ago

Hey @AndreLouisCaron! I think I finally have the time to show this project some love.

While I have some ideas for updates (e.g. #13), first I'd like to plow through the tests for this plugin itself. I think that they are tied to the implementation too much in most places, which shows in the ubiquitous mocking of subprocess.check_output. Because of that, they will all probably break with any serious refactoring.

I think we can have a 100% coverage with like two tests that actually run docker_services and a couple tests for discrete functions. That should demonstrate that the plugin is actually working.

I plan to submit a couple of self-contained but chained (one depending upon the other) pull requests. And, of course, I'll be mindful of not breaking anything for the current users. What do you think about this whole thing?

I'm already tackling the problems (https://github.com/pytest-dev/pytest/issues/4193), but I wanted to hear your thoughts on that before I go far ahead.

AndreLouisCaron commented 5 years ago

Hey @butla,

I'm not sure why I wrote the initial tests using a mock of check_output(). It was probably a combination of build speed and ease of testing. Given the relatively low activity on this project, I don't think that really makes sense anymore.

If you want to rewrite the tests without the mocks and actually invoke the docker executable, I'm OK with that :-)

butla commented 5 years ago

That's great to hear. I should start submitting pull requests this week.

butla commented 5 years ago

I'll get rid of some of tests alongside refactors necessary to be able to show container output during failures. Which will be optional to not break anything for anybody, of course. I think this library can be taken to 1.0 after that :)

AndreLouisCaron commented 5 years ago

Hey there :-)

Thanks for all your recent contributions. Are you expecting to send in more changes or should I release the current master to PyPI?

butla commented 5 years ago

I had plans to finally introduce the features I wanted and refactoring the core a bit, but I just got a new job and need to handle relocation :) Maybe I'll have something in a week. As for releasing to PyPI - the only thing it'd do right now is getting rid of the fallback, but actually that might be bad. It'll leave time for people to submit a bug if they are still using it before I get to work on the core :)

butla commented 5 years ago

I mean that realasing now would be NOT bad :)

AndreLouisCaron commented 5 years ago

Yeah, I actually plan on trying this on ou build machines at work since that's the context for which this feature was originally written. I'm quite busy ATM, but I'll get around to it soon and keep you posted!

Congrats on the new job and good luck with relocation :-)