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

Add an avail get_data test. #158

Merged
merged 1 commit into from
Oct 13, 2016
Merged

Conversation

jshaughn
Copy link
Collaborator

It just tests an extra code path that we didn't have.

@pilhuhn please review.

@josejulio
Copy link
Member

Rubocop strikes again!
@jshaughn could you please check?

@coveralls
Copy link

coveralls commented Oct 10, 2016

Coverage Status

Coverage increased (+0.04%) to 95.975% when pulling 21c7217 on jshaughn:avail-getdata into b4cf17e on hawkular:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 95.975% when pulling 21c7217 on jshaughn:avail-getdata into b4cf17e on hawkular:master.

@coveralls
Copy link

coveralls commented Oct 10, 2016

Coverage Status

Coverage increased (+0.04%) to 95.975% when pulling 21c7217 on jshaughn:avail-getdata into b4cf17e on hawkular:master.

@josejulio
Copy link
Member

Travis is complaining about the cassette metrics_services/Templates/Mixed_metrics/Should_requests_raw_data_for_multiple_metrics.yml

I'm not really sure if we should keep metrics_0_0_16 and metrics_services.
It seems that we are using them both alike.
// cc: @pilhuhn @rubenvp8510

@rubenvp8510
Copy link
Contributor

I think at this point we can remove metrics_0_0_16 and leave only metric_services.

This was for the testing of transition from old versions of metrics to metrics 0.0.16 (mostly endpoint changes). but because now metric_services test against recent versions of metrics, this doesn't make sense anymore.

@pilhuhn
Copy link
Member

pilhuhn commented Oct 12, 2016

I think it is ok to treat metrics016 and -services as the same.
We still need to keep metric08 as this is what is in openshift 3.3 and earlier

@josejulio
Copy link
Member

I'll fix that and post a PR soon.

@@ -135,6 +135,7 @@
id1 = SecureRandom.uuid
id2 = SecureRandom.uuid
id3 = SecureRandom.uuid
id4 = SecureRandom.uuid

ids = [id1, id2, id3]
bindings = { id1: id1, id2: id2, id3: id3 }
Copy link
Member

Choose a reason for hiding this comment

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

Could you please add id4 to this hash (bindings)?
It will help VCR do its replace-template-magic-thing.

@coveralls
Copy link

coveralls commented Oct 13, 2016

Coverage Status

Coverage increased (+0.006%) to 95.948% when pulling b84299b on jshaughn:avail-getdata into 1ea3053 on hawkular:master.

@jshaughn
Copy link
Collaborator Author

@josejulio, This dumb PR is finally green :) Please make it go away unless you see something else that needs to be fixed. Thanks, Jay

@josejulio josejulio merged commit f2dc1cd into hawkular:master Oct 13, 2016
@josejulio
Copy link
Member

Thank you!

@rubenvp8510 rubenvp8510 modified the milestone: 2.9.0 Mar 1, 2017
@jshaughn jshaughn deleted the avail-getdata branch November 3, 2017 14:06
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.

None yet

5 participants