Skip to content
This repository has been archived by the owner on Nov 23, 2020. It is now read-only.

Fix test time sensitivity when checking tasks #111

Merged
merged 1 commit into from Nov 10, 2014

Conversation

dcarley
Copy link
Contributor

@dcarley dcarley commented Nov 10, 2014

Integration tests which confirm atomicity and idempotency of updates by
checking the number of tasks generated during an operation were sensitive to
time differences between the clocks on the local machine and vCloud
Director.

This could be reproduced by putting your local clock forward one hour. You'd
get a test failure like the following:

1) Vcloud::EdgeGatewayServices Test EdgeGatewayServices with multiple services Check update is functional should only create one edgeGateway update task when updating the configuration
   Failure/Error: expect(task_list_after_update.size - task_list_before_update.size).to be(1)

     expected #<Fixnum:3> => 1
          got #<Fixnum:1> => 0

     Compared using equal?, which compares object identity,
     but expected and actual are not the same object. Use
     `expect(actual).to eq(expected)` if you don't care about
     object identity in this example.
   # ./spec/integration/edge_gateway/edge_gateway_services_spec.rb:44:in `block (4 levels) in <module:Vcloud>'

This is because it was using the local time to find all tasks that had
occurred since the beginning of that test and vCloud Director didn't know of
any tasks that had occurred in (what it considers to be) the future.

Fix this by using a previous Edge Gateway task as a marker and find all
tasks that have occurred since that. The timestamp of that last task is
generated by the clock available to vCloud Director, so any difference
between local and remote time is no longer an issue.

In most cases the previous task will be another one of our tests or the
reset_edge_gateway before block. The helper will raise an exception if
it's unable to find anything at all. We make sure that task is removed
from the elapsed results by matching it's unique href field.

The option pageSize 1 is used in the initial query to optimise the amount
of data we get from vCloud, because we only care about the single most
recent. This depends on a new release of vcloud-core for
gds-operations/vcloud-core#140 - without which it will fetch all results
one-by-one, very slowly.


Tests will fail until vcloud-core is merged/released.

@@ -57,34 +56,31 @@ module Vcloud
end

it "should not update the EdgeGateway again if the config hasn't changed" do
start_time = Time.now.getutc
task_list_before_update = get_all_edge_gateway_update_tasks_ordered_by_start_date_since_time(start_time)
task_last = IntegrationHelper.get_last_task(@test_params.edge_gateway)
Copy link
Contributor

Choose a reason for hiding this comment

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

Tiny point but might last_task be clearer here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Heh. It was originally last_task, then I changed it to task_last for symmetry with tasks_elapsed. Prefer readability over symmetry?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I reckon so.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@annashipman
Copy link
Contributor

👍 Looks good, much clearer as well. Will be nice to get rid of this annoying error.

Two tiny comments, otherwise happy to merge this once vCloud Core is released.

Integration tests which confirm atomicity and idempotency of updates by
checking the number of tasks generated during an operation were sensitive to
time differences between the clocks on the local machine and vCloud
Director.

This could be reproduced by putting your local clock forward one hour. You'd
get a test failure like the following:

    1) Vcloud::EdgeGatewayServices Test EdgeGatewayServices with multiple services Check update is functional should only create one edgeGateway update task when updating the configuration
       Failure/Error: expect(task_list_after_update.size - task_list_before_update.size).to be(1)

         expected #<Fixnum:3> => 1
              got #<Fixnum:1> => 0

         Compared using equal?, which compares object identity,
         but expected and actual are not the same object. Use
         `expect(actual).to eq(expected)` if you don't care about
         object identity in this example.
       # ./spec/integration/edge_gateway/edge_gateway_services_spec.rb:44:in `block (4 levels) in <module:Vcloud>'

This is because it was using the local time to find all tasks that had
occurred since the beginning of that test and vCloud Director didn't know of
any tasks that had occurred in (what it considers to be) the future.

Fix this by using a previous Edge Gateway task as a marker and find all
tasks that have occurred since that. The timestamp of that last task is
generated by the clock available to vCloud Director, so any difference
between local and remote time is no longer an issue.

In most cases the previous task will be another one of our tests or the
`reset_edge_gateway` before block. The helper will raise an exception if
it's unable to find anything at all. We make sure *that* task is removed
from the elapsed results by matching it's unique `href` field.

The option `pageSize 1` is used in the initial query to optimise the amount
of data we get from vCloud, because we only care about the single most
recent. This depends on a new release of vcloud-core for
gds-operations/vcloud-core#140 - without which it will fetch all results
one-by-one, very slowly.
@annashipman
Copy link
Contributor

👍

annashipman added a commit that referenced this pull request Nov 10, 2014
Fix test time sensitivity when checking tasks
@annashipman annashipman merged commit 43e70a1 into master Nov 10, 2014
@annashipman annashipman deleted the task_list_time_sensitive branch November 10, 2014 10:46
@mattbostock
Copy link
Contributor

🍆 Nice solution to use the last known task as a marker.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants