search

danielbachhuber / composer-lock-updater

Run composer-lock-updater in your CI system for bot-powered composer.lock pull requests.
MIT License
17 stars 7 forks source link

Add functional tests to the project #3

Open danielbachhuber opened 7 years ago

danielbachhuber commented 7 years ago

From @greg-1-anderson:

Functional tests for the basic functionality (pre this PR), it would be pretty easy. We could use composer create-project to clone a project that has out-of-date components, and then run clu on it. The Terminus Build Tools plugin uses similar techniques in its tests.

Functional tests for re-using a PR would be more difficult. Maybe we could cheat a bit to do this step. Create a PR per the above with clu; then, edit the composer.json to add a component that was not previously included in the system-under-test. At that point, running clu again will behave the same way that it would when there was an update. So, while we would not be testing the same use-case, we would be exercising the code in the same way, and I think that would be sufficient. Creating a mock packagist.org would be too much trouble, and mocking the API calls with something like VCR would also be more work, and less valuable.

danielbachhuber commented 5 years ago

@greg-1-anderson @ataylorme I spent a little bit of time thinking about this, and I think it's going to be hard to retrofit functional tests into the project without significant refactoring.

Essentially, we have two classes of interactions: system executables and remote services.

Some system executables could be run directly (e.g. git). Others, like hub, we could write simple mock equivalents for. But, in both cases, we'd need to touch a lot of code, replacing use of exec() with some wrapper, and breaking start() into more discrete, testable pieces.

For remote services (i.e. GitHub and GitLab), we could create a couple of repos with out-of-date composer.lock files. We might run into concurrency issues though, and there are a lot of variations I'd want to test.

Do you think it's worth the effort to embark upon this refactoring? I'm not totally convinced, given how infrequently this project changes. It might be simpler to just clean up and land #17

ataylorme commented 5 years ago

@danielbachhuber I don't think we need to test hub (or lab) functionality. Hub has it's own test suite. Likewise, hub is documented as a requirement so we can expect CLU users to ensure it is available.

I'm open to discussing testing remote services further (as Greg describes above) but don't want it to be a blocker.

Please move forward with cleaning up and merging #17

greg-1-anderson commented 5 years ago

Mocking is for unit tests or integration tests; my last sentence in the OP was a -1 on unit testing and a +1 on functional testing.

With functional testing, you'd just want to use a docker container that already had git and hub et. al. pre-installed, and then for each test you just run composer clu.

Certainly it takes time to set up tests; however, no refactoring is necessary to write functional tests.

danielbachhuber commented 5 years ago

With functional testing, you'd just want to use a docker container that already had git and hub et. al. pre-installed, and then for each test you just run composer clu.

Wouldn't we also need GitHub and GitLab accounts configured too? If you ran composer clu without the accounts configured, the test would fail?

I don't think we need to test hub (or lab) functionality. Hub has it's own test suite. Likewise, hub is documented as a requirement so we can expect CLU users to ensure it is available.

Sorry, to clarify, I mean testing the interactions between CLU and hub and lab (i.e. behavior when the exec() call is successful vs. behavior when there's various erred states).

Please move forward with cleaning up and merging #17

Sounds good, will do.

danielbachhuber commented 5 years ago

Now that I've made it through the other issues, if we're going to continue on with work like #20 and #21, then I think we need to take the time to refactor and make the whole thing more easily testable.

greg-1-anderson commented 5 years ago

Yeah, when I am writing functional tests for projects like this, I usually make actual GitHub organizations and create actual GitHub sites as test fixtures.

It is totally reasonable to decide that it's too much work to do that. Trying to mock the API calls would also be a lot of work, and would not be any value in some circumstances e.g. when changes to the service break the tool.

ataylorme commented 5 years ago

Now that I've made it through the other issues, if we're going to continue on with work like #20 and #21, then I think we need to take the time to refactor and make the whole thing more easily testable.

I opened those issue as room for improvement, they don't need to be the immediate priority. #17 is needed for our GitLab work elsewhere so let's take care of that then come back to discussing adding tests/refactoring

AlexSkrypnyk commented 5 years ago

It may be possible to use Mock Server and mock all interactions with Hub to avoid creating real accounts.