Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mock ansible invocation in unit test #123

Open
gciavarrini opened this issue Mar 30, 2022 · 7 comments
Open

Mock ansible invocation in unit test #123

gciavarrini opened this issue Mar 30, 2022 · 7 comments
Labels
good first issue Good for newcomers

Comments

@gciavarrini
Copy link
Collaborator

At the moment ansible must be installed to successfully run unit tests of package ansible because they rely on real invocations of ansible-playbook command (via go-ansible library).

Unit-test shouldn't depend on external resources.

We need to mock them or to present an API that will encapsulate the actual call to the external service.

@gciavarrini gciavarrini added the good first issue Good for newcomers label Mar 30, 2022
@gabriel-farache
Copy link
Contributor

Are tests in ansible_manager_test.go really unit tests and not integrations tests?
The unit test would be to check if we go inside the HandlePlaybook() and ExecutePendingPlaybooks() that are supposed to execute the ansible playbook.
By checking the output of the execution of the playbook, it looks like an integration test
One easy way would be to run tests in docker container in which all external dependencies are installed for integrations tests:

  • So in the Makefile add a target to run integration tests that will executes the ginkgo in the docker.
  • The target should be able to take specific folders to run tests on in case the user does not want to run all tests but only a subset

What do you think?

@gciavarrini
Copy link
Collaborator Author

You're right: they are integration tests. The goals of the issue is to move them in the integration test section (maybe we can reword the issue description).

Regarding implementation I think @machacekondra can help :)

@gabriel-farache
Copy link
Contributor

Ok so the goal of this issue is to create a new folder test/integration and to move the ansible tests in it, correct?
Then add a target in the makefile to run those tests (see my above comment about how I would do it), @machacekondra any advices?

@gciavarrini
Copy link
Collaborator Author

The test/e2e folder already exists.

For this issue we should mock ansible invocation and refactor the current unit tests to be "proper" unit test (https://issues.redhat.com/browse/ECOPROJECT-691).

We also planned to add e2e CI test but, I think, It can be another issue/PR .

@gabriel-farache
Copy link
Contributor

There is no e2e folder in the device-worker project in the main branch at least and the JIRA ticket has "flotta operator" as component :)
The current e2e tests are not executed in the CI currently? Because I see a target in the Makefile
So I am a little confused to what should be done here: move the tests to correct folder and maybe create a Maefile target to run tests in docker container (at least for ansible tests) to avoid being dependent of the platform, or move the test and then create unit tests?

@gciavarrini
Copy link
Collaborator Author

There is no e2e folder in the device-worker project

AFAIK, the e2e test should be part of the operator. The folder test\e2e is there.

the JIRA ticket has "flotta operator" as component :)

Fixed! Thanks :)

So I am a little confused to what should be done here

Actual test needs to be part of e2e: I've no good suggestion on the implementation for you but I think they should be part of the operator, so we need to wait for the implementation step (we are still defining the CRDs).

IMO, for this specific issue the goal is to mock ansible invocation in the current available tests. This step it's necessary to make unit test independent from the availability of ansible command and to ease the writing of proper unit test.

Having said that, if anyone has different opinions on this issue I'd like to hear them! :)

@gabriel-farache
Copy link
Contributor

Yes, integrations tests should be in e2e folder, I agree. But in the device-worker project, such folder does not exists that why I suggested to create one in the project to put the current ansible tests that are integration tests in it.

As for mocking the AnsiblePlaybookCmd (as , from my understanding this is the playbookCmd that is used to used to execute the ansible playbook) in the dispatcher I agree that mocking it would be useful

Maybe we could discuss this at tomorrow team meeting if that is fine with you and if it fits in the agenda

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants