Code review comment for ppa-dev-tools:test-create-config

Revision history for this message
Alberto Contreras (aciba) wrote :

Thanks, Bryce, for adding unit tests. The changes look good.

I have left some nits and two questions / concerns.

1) The main one is the inclusion of the mocking machinery in `Ppa.archive`.

This helps us to unit test the code and it is okay as is.

But, adds new code that it is not needed at runtime and could cause problems in the future. In this case, this is probably an exaggeration, as the modifications are simple and easy to track. But if we establish this pattern of allowing testing code within production code, things could get complicated and bite us at some point.

Have we thought about using other alternatives, as using `pytest.fixtures` and/or `unittests.mock` in the long term? Or are there restrictions to use those that I do not see?

2) The xfail comment.

review: Needs Information

« Back to merge proposal