Skip to content

(POOLER-73) Update unit tests prior to refactoring#182

Merged
shrug merged 35 commits into
puppetlabs:masterfrom
glennsarti:ticket/master/POOLER-73-add-unit-tests
Mar 2, 2017
Merged

(POOLER-73) Update unit tests prior to refactoring#182
shrug merged 35 commits into
puppetlabs:masterfrom
glennsarti:ticket/master/POOLER-73-add-unit-tests

Conversation

@glennsarti

@glennsarti glennsarti commented Feb 9, 2017

Copy link
Copy Markdown
Contributor

Previously all of the spec tests for VM Pooler were all together in the specs
directory. However some tests require a working local Redis server to operate
and others instead mock all external dependencies. This commit splits the test
files between unit and integration, where integration tests require a working
Redis instance, and unit tests do not. This commit also removes the root
vmpooler directory as it is not required. The tests rake test still operates
correctly.

Previously there were not enough unit tests to assure that refactoring will not unnecessarily fail. These commits add more spec tests to assist in refactoring in POOLER-70

  • check_pending_vm(vm, pool, timeout, vsphere)

  • open_socket(host, domain=nil, timeout=5, port=22, &block)

  • _check_pending_vm(vm, pool, timeout, vsphere)

  • remove_nonexistent_vm(vm, pool)

  • fail_pending_vm(vm, pool, timeout, exists=true)

  • move_pending_vm_to_ready(vm, pool, host)

  • check_ready_vm(vm, pool, ttl, vsphere)

  • check_running_vm(vm, pool, ttl, vsphere)

  • _check_running_vm(vm, pool, ttl, vsphere)

  • move_vm_queue(pool, vm, queue_from, queue_to, msg)

  • clone_vm(template, folder, datastore, target, vsphere)

  • destroy_vm(vm, pool, vsphere)

  • create_vm_disk(vm, disk_size, vsphere)

  • _create_vm_disk(vm, disk_size, vsphere)

  • create_vm_snapshot(vm, snapshot_name, vsphere)

  • _create_vm_snapshot(vm, snapshot_name, vsphere)

  • revert_vm_snapshot(vm, snapshot_name, vsphere)

  • _revert_vm_snapshot(vm, snapshot_name, vsphere)

  • check_disk_queue

  • _check_disk_queue(vsphere)

  • check_snapshot_queue

  • _check_snapshot_queue(vsphere)

  • migration_limit(migration_limit)

  • migrate_vm(vm, pool, vsphere)

  • _migrate_vm(vm, pool, vsphere)

  • get_vm_host_info(vm_object)

  • remove_vmpooler_migration_vm(pool, vm)

  • migrate_vm_and_record_timing(vm_object, vm_name, pool, host, source_host_name, dest_host_name, vsphere)

  • check_pool(pool)

  • _check_pool(pool, vsphere)

  • execute!

  • Refactor for Mock Redis (instead of using method mocks)

  • Refactor for a better way to expect log messages? ( Will do in a later PR)

@glennsarti

Copy link
Copy Markdown
Contributor Author

This is a WIP PR for review.

@glennsarti glennsarti force-pushed the ticket/master/POOLER-73-add-unit-tests branch 7 times, most recently from 5941e8a to 7464bb9 Compare February 14, 2017 04:50
Previously all of the spec tests for VM Pooler were all together in the specs
directory.  However some tests require a working local Redis server to operate
and other instead mock all external dependencies.  This commit splits the test
files between unit and integration, where integration tests require a working
Redis instance, and unit tests do not.  This commit also removes the root
`vmpooler` directory as it is not required.  The tests rake test still operates
correctly.

This commit also adds the mock_redis library for testing for the pool_manager.
@glennsarti glennsarti force-pushed the ticket/master/POOLER-73-add-unit-tests branch from 7464bb9 to 7c035ad Compare February 17, 2017 21:05
This commit adds the following test helpers:
- MockFindFolder
  Returns an mock result object from calling `Vmpooler::VsphereHelper.find_folder(foldername)`
- Use MockRedis instead of using method stubs
- MockLogger
  Creates an object which looks like the VMPooler::Logger object but just
  ignores all messages.  This stops the proliferation of allow(logger).to ....
  expectations in tests
- create_completed_vm
  Creates the required redis information for a completed VM
- create_discovered_vm
  Creates the required redis information for a newly discovered VM
- snapshot_revert_vm
  Creates the required redis information to revert a snapshot for a VM
- disk_task_vm
   Creates the required redis information to add a disk addition task to a VM
Add spec tests for check_pending_vm
 Add spec tests for open_socket
(POOLER-73) Modify spec tests for _check_pending_vm
Add spec tests for remove_nonexistent_vm
Modify spec tests for fail_pending_vm
Modify spec tests for move_pending_vm_to_ready
Add spec tests for check_ready_vm
Add spec tests for check_running_vm
Modify spec tests for _check_running_vm
Add spec tests for move_vm_queue
Add spec tests for clone_vm
Add spec tests for destroy_vm
Add spec tests for get_vm_host_info
Add spec tests for execute!

Previously the execute! method would execute the loop indefinitely as it did not
have a terminating condition.  This made it impossible to test.  This commit
modifies the execute! method so that it can take a maxloop and delay parameter
so that it can be tested.
Add spec tests for check_pool

Previously the check_pool method would execute the loop indefinitely as it did not
have a terminating condition.  This made it impossible to test.  This commit
modifies the check_pool method so that it can take a maxloop and delay parameter
so that it can be tested.
The tests for #move_running_to_completed are already tested in the
Add spec tests for _check_pool
This commit removes the #_stats_running_ready tests as they are covered in the
_check_pool tests.
Add spec tests for create_vm_disk
Add spec tests for create_vm_snapshot
Add spec tests for revert_vm_snapshot
Add spect tests for migrate_vm
Add spec tests for migration_limit
Add spec tests for remove_vmpooler_migration_vm
Add spec tests for migrate_vm_and_record_timing
Add spec tests for check_disk_queue

Previously the check_disk_queue method would execute the loop indefinitely as it
did not have a terminating condition.  This made it impossible to test.  This
commit modifies the check_disk_queue method so that it can take a maxloop and
delay parameter so that it can be tested.
Add spec tests for check_snapshot_queue

Previously the check_snapshot_queue method would execute the loop indefinitely
as it did not have a terminating condition.  This made it impossible to test.
This commit modifies the check_snapshot_queue method so that it can take a
maxloop and delay parameter so that it can be tested.
Add spec tests for _check_snapshot_queue
Add spec tests for _check_disk_queue
Add spec tests for _create_vm_disk
Modify spec tests for _create_vm_snapshot
Modify spec tests for _revert_vm_snapshot
Modify spec tests for _migrate_vm
@glennsarti glennsarti force-pushed the ticket/master/POOLER-73-add-unit-tests branch from 7c035ad to e783e93 Compare February 17, 2017 23:51
@glennsarti glennsarti changed the title {WIP}(POOLER-73) Update unit tests prior to refactoring (POOLER-73) Update unit tests prior to refactoring Feb 17, 2017
@glennsarti

Copy link
Copy Markdown
Contributor Author

This PR is ready for review.

@glennsarti

Copy link
Copy Markdown
Contributor Author

While the number of tests isn't always a good measure, have gone from 139 to 349 tests.

@glennsarti

Copy link
Copy Markdown
Contributor Author

Waiting on #187 to be merged to clear Travis failure

@shrug

shrug commented Feb 28, 2017

Copy link
Copy Markdown

#187 has been merged, I think travis is still recovering from AWS issues earlier

@glennsarti

Copy link
Copy Markdown
Contributor Author

yeah...I'll rekick the job tomorrow.

end
end
end
end

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit messages make it sound like it should all be test changes but these look like code changes. Is this expected?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Perhaps my commit title is misleading.

But in the commit msg itself:

Previously the check_pool method would execute the loop indefinitely as it did not
have a terminating condition.  This made it impossible to test.  This commit
modifies the check_pool method so that it can take a maxloop and delay parameter
so that it can be tested.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, cool. 👍

@shrug shrug merged commit 0754f86 into puppetlabs:master Mar 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants