Skip to content

Conversation

@Temikus
Copy link
Member

@Temikus Temikus commented Apr 11, 2018

Fixing up tests so we can rely on CI for breakages. Starting with a rather embarrassing regression I introduced.

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Error: obsolete parameter MaxLineLength (for Style/IfUnlessModifier) found in...
Error: obsolete parameter MaxLineLength (for Style/IfUnlessModifier) found in .rubocop.yml
`Style/IfUnlessModifier: MaxLineLength` has been removed. Use `Metrics/LineLength: Max` instead

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Error: obsolete parameter MaxLineLength (for Style/IfUnlessModifier) found in...
Error: obsolete parameter MaxLineLength (for Style/IfUnlessModifier) found in .rubocop.yml
`Style/IfUnlessModifier: MaxLineLength` has been removed. Use `Metrics/LineLength: Max` instead

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Error: obsolete parameter MaxLineLength (for Style/IfUnlessModifier) found in...
Error: obsolete parameter MaxLineLength (for Style/IfUnlessModifier) found in .rubocop.yml
`Style/IfUnlessModifier: MaxLineLength` has been removed. Use `Metrics/LineLength: Max` instead

The option to submit a P12 key is no longer available.

See fog@f1607b4
Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Error: obsolete parameter MaxLineLength (for Style/IfUnlessModifier) found in...
Error: obsolete parameter MaxLineLength (for Style/IfUnlessModifier) found in .rubocop.yml
`Style/IfUnlessModifier: MaxLineLength` has been removed. Use `Metrics/LineLength: Max` instead

assert_kind_of(Fog::Compute::Google::Real, c)
end


Choose a reason for hiding this comment

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

Layout/TrailingWhitespace: Trailing whitespace detected.

@Temikus
Copy link
Member Author

Temikus commented Apr 11, 2018

Ahhhh, so it was a parameter, not a rule...

@icco FYI I fixed hound.

Temikus added 2 commits April 11, 2018 22:34
Validation changed from client-side to method in:
fog@9e13610

Adjusting the test to match.
API library method changed to parameters instead of hashes.
Now the resources are cleaning up correctly.
list_resp = @client.monitoring.list_project_metric_descriptors(
:filter => "metric.type = starts_with(\"#{TEST_METRIC_PREFIX}\")"
"projects/#{@client.project}",
filter: "metric.type = starts_with(\"#{TEST_METRIC_PREFIX}\")"

Choose a reason for hiding this comment

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

Style/HashSyntax: Use hash rockets syntax.

API Client now returns a bad request exception if the metric is
not yet created, so needed to insert a Retriable step.
# metric hasn't yet been created
Retriable.retriable(on: {Google::Apis::ClientError => /The provided filter doesn't refer to any known metric./},
tries: 2,
base_interval: 30) do

Choose a reason for hiding this comment

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

Style/HashSyntax: Use hash rockets syntax.

# Retriable is used instead of wait_for due to API client returning Google::Apis::ClientError: badRequest if the
# metric hasn't yet been created
Retriable.retriable(on: {Google::Apis::ClientError => /The provided filter doesn't refer to any known metric./},
tries: 2,

Choose a reason for hiding this comment

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

Style/HashSyntax: Use hash rockets syntax.

# Wait for metric to be created
# Retriable is used instead of wait_for due to API client returning Google::Apis::ClientError: badRequest if the
# metric hasn't yet been created
Retriable.retriable(on: {Google::Apis::ClientError => /The provided filter doesn't refer to any known metric./},

Choose a reason for hiding this comment

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

Style/HashSyntax: Use hash rockets syntax.
Layout/SpaceInsideHashLiteralBraces: Space inside { missing.
Layout/SpaceInsideHashLiteralBraces: Space inside } missing.

tries: 2,
base_interval: 30) do
@client.list_timeseries(
:filter => "metric.type = \"#{metric_type}\"",

Choose a reason for hiding this comment

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

Layout/FirstParameterIndentation: Indent the first parameter one step more than the start of the previous line.

# metric hasn't yet been created
Retriable.retriable(on: {Google::Apis::ClientError => /The provided filter doesn't refer to any known metric./},
tries: 2,
base_interval: 30) do

Choose a reason for hiding this comment

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

Style/HashSyntax: Use hash rockets syntax.

# Retriable is used instead of wait_for due to API client returning Google::Apis::ClientError: badRequest if the
# metric hasn't yet been created
Retriable.retriable(on: {Google::Apis::ClientError => /The provided filter doesn't refer to any known metric./},
tries: 2,

Choose a reason for hiding this comment

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

Style/HashSyntax: Use hash rockets syntax.

# Wait for metric to be created
# Retriable is used instead of wait_for due to API client returning Google::Apis::ClientError: badRequest if the
# metric hasn't yet been created
Retriable.retriable(on: {Google::Apis::ClientError => /The provided filter doesn't refer to any known metric./},

Choose a reason for hiding this comment

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

Style/HashSyntax: Use hash rockets syntax.
Layout/SpaceInsideHashLiteralBraces: Space inside { missing.
Layout/SpaceInsideHashLiteralBraces: Space inside } missing.

@Temikus
Copy link
Member Author

Temikus commented Apr 12, 2018

Ok, now we’re down to 6 errors versus 30something when I got started. Still working on it.

Temikus added 2 commits April 12, 2018 20:16
Instances now appear to enter STAGING almost immidiately, making
checks for PROVISIONING fail.

See: https://cloud.google.com/compute/docs/instances/checking-instance-status
Instances are now STAGING almost immidiately, so the wait_for
timed out.
# metric hasn't yet been created
Retriable.retriable(on: {Google::Apis::ClientError => /The provided filter doesn't refer to any known metric./},
tries: 2,
base_interval: 30) do

Choose a reason for hiding this comment

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

Style/HashSyntax: Use hash rockets syntax.

# Retriable is used instead of wait_for due to API client returning Google::Apis::ClientError: badRequest if the
# metric hasn't yet been created
Retriable.retriable(on: {Google::Apis::ClientError => /The provided filter doesn't refer to any known metric./},
tries: 2,

Choose a reason for hiding this comment

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

Style/HashSyntax: Use hash rockets syntax.

@Temikus
Copy link
Member Author

Temikus commented Apr 12, 2018

Network model gateway and ipv4 range fields seem to not be functional due to a typo in the GoogleAPI lib: googleapis/google-api-ruby-client#666

I'm not entirely sure how to populate the data properly, since the aliases do not seem to work when instantiating a class, i.e.:

network = service.get_network(identity).to_h
[3] pry(#<Fog::Compute::Google::Networks>)> network = service.get_network(identity).to_h
=> {:name=>"fog-test-networkscol-842a61e2-15f3-4179-97ff-7c7a7ffada8b",
 :creation_timestamp=>"2018-04-12T04:01:42.889-07:00",
 :id=>7840112885788476025,
 :kind=>"compute#network",
 :routing_config=>{:routing_mode=>"REGIONAL"},
 :self_link=>"https://www.googleapis.com/compute/v1/projects/myproject/global/networks/fog-test-networkscol-842a61e2-15f3-4179-97ff-7c7a7ffada8b",
 :i_pv4_range=>"10.240.0.0/16",
 :gateway_i_pv4=>"10.240.0.1"}

[4] pry(#<Fog::Compute::Google::Networks>)> new(network)
=>   <Fog::Compute::Google::Network
    name="fog-test-networkscol-842a61e2-15f3-4179-97ff-7c7a7ffada8b",
    auto_create_subnetworks=nil,
    creation_timestamp="2018-04-12T04:01:42.889-07:00",
    description=nil,
    gateway_ipv4=nil,
    ipv4_range=nil,
    id=7840112885788476025,
    kind="compute#network",
    peerings=nil,
    routing_config={:routing_mode=>"REGIONAL"},
    self_link="https://www.googleapis.com/compute/v1/projects/myproject/global/networks/fog-test-networkscol-842a61e2-15f3-4179-97ff-7c7a7ffada8b",
    subnetworks=nil
  >

😕 I'll try tackling this tomorrow a bit more.

# metric hasn't yet been created
Retriable.retriable(on: {Google::Apis::ClientError => /The provided filter doesn't refer to any known metric./},
tries: 2,
base_interval: 30) do

Choose a reason for hiding this comment

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

Style/HashSyntax: Use hash rockets syntax.

# Retriable is used instead of wait_for due to API client returning Google::Apis::ClientError: badRequest if the
# metric hasn't yet been created
Retriable.retriable(on: {Google::Apis::ClientError => /The provided filter doesn't refer to any known metric./},
tries: 2,

Choose a reason for hiding this comment

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

Style/HashSyntax: Use hash rockets syntax.

url_map.merge(:name => url_map_name)
)
@compute.insert_url_map(@project, url_map_obj)
# HACK - Currently URL map insert may fail even though the backend

Choose a reason for hiding this comment

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

Style/CommentAnnotation: Annotation keywords like HACK should be all upper case, followed by a colon, and a space, then a note describing the problem.

@Temikus
Copy link
Member Author

Temikus commented Apr 13, 2018

Ok, one test left to fix.

I'll address Hound with the final commit to not pepper them everywhere and see if anything's failing after changing the hash format.

# metric hasn't yet been created
Retriable.retriable(on: {Google::Apis::ClientError => /The provided filter doesn't refer to any known metric./},
tries: 2,
base_interval: 30) do

Choose a reason for hiding this comment

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

Style/HashSyntax: Use hash rockets syntax.

# Retriable is used instead of wait_for due to API client returning Google::Apis::ClientError: badRequest if the
# metric hasn't yet been created
Retriable.retriable(on: {Google::Apis::ClientError => /The provided filter doesn't refer to any known metric./},
tries: 2,

Choose a reason for hiding this comment

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

Style/HashSyntax: Use hash rockets syntax.

# Now that we have an address, we can create a server using the static ip
server_name = new_resource_name
client.servers.create(

Choose a reason for hiding this comment

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

Layout/TrailingWhitespace: Trailing whitespace detected.

# metric hasn't yet been created
Retriable.retriable(on: {Google::Apis::ClientError => /The provided filter doesn't refer to any known metric./},
tries: 2,
base_interval: 30) do

Choose a reason for hiding this comment

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

Style/HashSyntax: Use hash rockets syntax.

# Retriable is used instead of wait_for due to API client returning Google::Apis::ClientError: badRequest if the
# metric hasn't yet been created
Retriable.retriable(on: {Google::Apis::ClientError => /The provided filter doesn't refer to any known metric./},
tries: 2,

Choose a reason for hiding this comment

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

Style/HashSyntax: Use hash rockets syntax.

@Temikus
Copy link
Member Author

Temikus commented Apr 13, 2018

Ok, so now only some flaky tests are left. Most notably the stackdriver API:

 1) Error:
TestMetricDescriptors#test_multiple_timeseries:
Google::Apis::ClientError: badRequest: The provided filter doesn't refer to any known metric.
    /var/lib/gems/2.3.0/gems/google-api-client-0.19.8/lib/google/apis/core/http_command.rb:218:in `check_status'
    /var/lib/gems/2.3.0/gems/google-api-client-0.19.8/lib/google/apis/core/api_command.rb:116:in `check_status'
    /var/lib/gems/2.3.0/gems/google-api-client-0.19.8/lib/google/apis/core/http_command.rb:183:in `process_response'
    /var/lib/gems/2.3.0/gems/google-api-client-0.19.8/lib/google/apis/core/http_command.rb:299:in `execute_once'
    /var/lib/gems/2.3.0/gems/google-api-client-0.19.8/lib/google/apis/core/http_command.rb:104:in `block (2 levels) in execute'
    /var/lib/gems/2.3.0/gems/retriable-3.1.1/lib/retriable.rb:61:in `block in retriable'
    /var/lib/gems/2.3.0/gems/retriable-3.1.1/lib/retriable.rb:57:in `times'
    /var/lib/gems/2.3.0/gems/retriable-3.1.1/lib/retriable.rb:57:in `retriable'
    /var/lib/gems/2.3.0/gems/google-api-client-0.19.8/lib/google/apis/core/http_command.rb:101:in `block in execute'
    /var/lib/gems/2.3.0/gems/retriable-3.1.1/lib/retriable.rb:61:in `block in retriable'
    /var/lib/gems/2.3.0/gems/retriable-3.1.1/lib/retriable.rb:57:in `times'
    /var/lib/gems/2.3.0/gems/retriable-3.1.1/lib/retriable.rb:57:in `retriable'
    /var/lib/gems/2.3.0/gems/google-api-client-0.19.8/lib/google/apis/core/http_command.rb:93:in `execute'
    /var/lib/gems/2.3.0/gems/google-api-client-0.19.8/lib/google/apis/core/base_service.rb:360:in `execute_or_queue_command'
    /var/lib/gems/2.3.0/gems/google-api-client-0.19.8/generated/google/apis/monitoring_v3/service.rb:695:in `list_project_time_series'
    /tmp/build/453021c2/src/fog-google/lib/fog/google/requests/monitoring/list_timeseries.rb:42:in `list_timeseries'
    /tmp/build/453021c2/src/fog-google/test/integration/monitoring/test_timeseries.rb:131:in `test_multiple_timeseries'

It's so flaky I think I just need to make a generic wrapper around it in the request itself.

I'll sleep on it.

@Temikus
Copy link
Member Author

Temikus commented Apr 13, 2018

Apparently this is a known problem with stackdriver metric, sometimes it requires 80+ seconds for the metric to become available:
GoogleCloudPlatform/openjdk-runtime#127

I'll try wrapping everything in backoff Retriable unless I come up with any new ideas.

Temikus added 2 commits April 14, 2018 12:43
This is an attempt to resolve clashing tests between addresses
and disks models due to them having a similar prefix and trying
to delete eachother's leftover resources.
)
series = Retriable.retriable(on: {Google::Apis::ClientError => NOT_READY_REGEX},
tries: RETRIABLE_TRIES,
base_interval: RETRIABLE_BASE_INTERVAL) do

Choose a reason for hiding this comment

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

Style/HashSyntax: Use hash rockets syntax.

:interval => interval
)
series = Retriable.retriable(on: {Google::Apis::ClientError => NOT_READY_REGEX},
tries: RETRIABLE_TRIES,

Choose a reason for hiding this comment

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

Style/HashSyntax: Use hash rockets syntax.

],
:interval => interval
)
series = Retriable.retriable(on: {Google::Apis::ClientError => NOT_READY_REGEX},

Choose a reason for hiding this comment

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

Style/HashSyntax: Use hash rockets syntax.
Layout/SpaceInsideHashLiteralBraces: Space inside { missing.
Layout/SpaceInsideHashLiteralBraces: Space inside } missing.

)
resp = Retriable.retriable(on: {Google::Apis::ClientError => NOT_READY_REGEX},
tries: RETRIABLE_TRIES,
base_interval: RETRIABLE_BASE_INTERVAL) do

Choose a reason for hiding this comment

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

Layout/AlignHash: Align the elements of a hash literal if they span more than one line.
Style/HashSyntax: Use hash rockets syntax.

:page_size => 1
)
resp = Retriable.retriable(on: {Google::Apis::ClientError => NOT_READY_REGEX},
tries: RETRIABLE_TRIES,

Choose a reason for hiding this comment

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

Layout/AlignHash: Align the elements of a hash literal if they span more than one line.
Style/HashSyntax: Use hash rockets syntax.

@client.create_timeseries(:timeseries => timeseries)
Retriable.retriable(on: Google::Apis::ServerError,
tries: RETRIABLE_TRIES,
base_interval: RETRIABLE_BASE_INTERVAL) do

Choose a reason for hiding this comment

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

Style/HashSyntax: Use hash rockets syntax.


@client.create_timeseries(:timeseries => timeseries)
Retriable.retriable(on: Google::Apis::ServerError,
tries: RETRIABLE_TRIES,

Choose a reason for hiding this comment

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

Style/HashSyntax: Use hash rockets syntax.

end

@client.create_timeseries(:timeseries => timeseries)
Retriable.retriable(on: Google::Apis::ServerError,

Choose a reason for hiding this comment

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

Style/HashSyntax: Use hash rockets syntax.


series = Retriable.retriable(on: {Google::Apis::ClientError => NOT_READY_REGEX},
tries: RETRIABLE_TRIES,
base_interval: RETRIABLE_BASE_INTERVAL) do

Choose a reason for hiding this comment

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

Layout/AlignHash: Align the elements of a hash literal if they span more than one line.
Style/HashSyntax: Use hash rockets syntax.

end

series = Retriable.retriable(on: {Google::Apis::ClientError => NOT_READY_REGEX},
tries: RETRIABLE_TRIES,

Choose a reason for hiding this comment

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

Layout/AlignHash: Align the elements of a hash literal if they span more than one line.
Style/HashSyntax: Use hash rockets syntax.

@Temikus
Copy link
Member Author

Temikus commented Apr 14, 2018

All tests pass \o/

tries: RETRIABLE_TRIES,
base_interval: RETRIABLE_BASE_INTERVAL) do
@client.list_timeseries(
:filter => "metric.type = \"#{metric_type}\"",

Choose a reason for hiding this comment

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

Layout/FirstParameterIndentation: Indent the first parameter one step more than the start of the previous line.

}
)
# Wait for metric to be created
Retriable.retriable(on: {Google::Apis::ClientError => NOT_READY_REGEX},

Choose a reason for hiding this comment

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

Layout/SpaceInsideHashLiteralBraces: Space inside { missing.
Layout/SpaceInsideHashLiteralBraces: Space inside } missing.

@Temikus
Copy link
Member Author

Temikus commented Apr 16, 2018

Reran 2 times, some additional flakiness is left. I'll attempt to debug.
Possibly will set operations for model creation to sync instead of async. That should help with PD flakes but will somewhat increase the test suite run time.

@Temikus Temikus changed the title WIP - Fixing tests Fixing tests Apr 25, 2018
@Temikus
Copy link
Member Author

Temikus commented Apr 25, 2018

Ok, I'll merge this to be able to test other PR's and open a new branch to fix the leftover disk flakiness.

@Temikus Temikus merged commit a652b02 into fog:master Apr 25, 2018
@icco
Copy link
Member

icco commented Apr 30, 2018

Thanks for this!

@Temikus Temikus deleted the test_fixup0 branch March 20, 2019 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants