test_engine_service tests are racy

[Steven Hardy]

Several of these tests rely on running code via eventlet threads scheduled
via the engine service ThreadGroupManager.

In some of these cases, we then go on to make assertions based on assumptions that the threads have completed
 (some of them are actually wrong assertions which test the thread is *not* completed!).

Even in the cases where wait() is called, we're introducing undue complexity, given that we just want to run the function (or test that it will be run) in the "unit" tests.

Examples:

https://github.com/openstack/heat/blob/master/heat/tests/test_engine_service.py#L761 (check for IN_PROGRESS will race the thread running and moving the stack to COMPLETE)

https://github.com/openstack/heat/blob/master/heat/tests/test_engine_service.py#L746 (check for persisted data in the DB assumes the thread has run sufficiently to persist the state, e.g the opposite of the above assertion!)

Similar issues throughout the file due to cut/paste badness.

[Steven Hardy (shardy)]

Actually same pattern in a few other places as well:

$ grep -R "create_periodic_tasks" ./* | grep -v pyc | grep -v bak
./engine/test_stack_create.py: self.man.create_periodic_tasks()
./engine/test_stack_snapshot.py: self.engine.create_periodic_tasks()
./engine/test_stack_events.py: self.eng.create_periodic_tasks()
./engine/test_stack_delete.py: self.man.create_periodic_tasks()
./engine/test_stack_action.py: self.man.create_periodic_tasks()
./test_engine_service.py: self.eng.create_periodic_tasks()
./test_metadata_refresh.py: self.man.create_periodic_tasks()

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/205163

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/205163
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=dc613540ddd406fb50f26204898ba561378ecfbd
Submitter: Jenkins
Branch: master

commit dc613540ddd406fb50f26204898ba561378ecfbd
Author: Steven Hardy <email address hidden>
Date: Thu Jul 23 11:34:06 2015 +0100

    Stop using eventlet threads in test_engine_service

    Currently we rely on actually running the tasks on in the
    stack ThreadGroup, which will be non-deterministic where
    wait() is not used (leading to some incorrect assertions)
    and unneccesarily complex even when using wait().

    So instead use a dummy implementation which allows for tasks
    to be either run immediately or just recorded so we can
    assert they would be started.

    Change-Id: I2a748cac50e380289c33eadfa1c53309f86ad145
    Partial-Bug: #1477484

[Steve Baker (steve-stevebaker)]

Is this still an issue?