-
-
Notifications
You must be signed in to change notification settings - Fork 22
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 measurements #21
Add measurements #21
Changes from 9 commits
f4709c3
78da473
7c7bd0d
c606f72
13b6607
821d4c2
2110e68
d890a98
500b403
d09f16f
84dda78
039a2fc
827a525
f838fe0
988122f
92f97cd
3da82b8
eab841f
aef424b
1a18cc2
6ed326a
8cf0c05
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -94,4 +94,68 @@ | |
end | ||
end | ||
end | ||
|
||
describe '#create_measurements' do | ||
it 'creates measurements', vcr: { cassette_name: 'stations/create_measurement_success' } do | ||
create_params = { | ||
"station_id": '5ed21a12cca8ce0001f1aef1', | ||
"dt": 1479817340, | ||
"temperature": 18.7, | ||
"wind_speed": 1.2, | ||
"wind_gust": 3.4, | ||
"pressure": 1021, | ||
"humidity": 87, | ||
"rain_1h": 2, | ||
"clouds": [ | ||
{ | ||
"condition": 'NSC' | ||
} | ||
] | ||
} | ||
data = client.create_measurements([create_params]) | ||
expect(data).to be_nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have to make sure the API was called, expect a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To clarify, we do this because the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Absolutely |
||
end | ||
|
||
context 'when station does not exist' do | ||
it 'raises error', vcr: { cassette_name: 'stations/create_measurement_failed_with_invalid_station' } do | ||
create_params = { | ||
"station_id": -1, | ||
"dt": 1479817340, | ||
"temperature": 18.7, | ||
"wind_speed": 1.2, | ||
"wind_gust": 3.4, | ||
"pressure": 1021, | ||
"humidity": 87, | ||
"rain_1h": 2, | ||
"clouds": [ | ||
{ | ||
"condition": 'NSC' | ||
} | ||
] | ||
} | ||
expect { client.create_measurements([create_params]) } | ||
.to raise_error(OpenWeather::Errors::Fault, /expected=string, got=number/) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an odd error, doesn't tell me that the station was invalid? Should this be a specialized error? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems to be what the API returns, at least with a number: Let me try with a string EDIT: You were right, good catch |
||
end | ||
end | ||
end | ||
|
||
describe '#get_measurements' do | ||
it 'gets measurements', vcr: { cassette_name: 'stations/get_measurement_success' } do | ||
data = client.get_measurements( | ||
station_id: '5ed21a12cca8ce0001f1aef1', | ||
type: 'd', | ||
limit: 100, | ||
from: 1469817340, | ||
to: 1591620047 | ||
) | ||
expect(data.size).to eq(1) | ||
expect(data.first['station_id']).to eq('5ed21a12cca8ce0001f1aef1') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like the request / response structures for measurements are somewhat different e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should make a new structure for anything unique. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There would potentially be two structures then, so OpenWeather::Stations::MeasurementRequest / MeasurementResponse? I could try to normalize the differences, but the response isn't as well documented as the request :/ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you should make one, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've made a best effort one :/ I guessed the properties for some of the "submodels" based on personal testing, but there are some weird ones e.g. I created a measurement with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's fine. You should double-check whether such documentation is really missing on their site, then send them an email asking to document the structures. |
||
end | ||
|
||
context 'without mandatory params' do | ||
wasabigeek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
it 'raises error' do | ||
expect { client.get_measurements(something: 'something') }.to raise_error(ArgumentError, /station_id, type, limit, from, to/) | ||
end | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mandatory -> required
Doesn't look like all these are required, so I'd say "The following parameters are required: ...".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are apparently all required, unfortunately >_< https://openweathermap.org/stations#get_measurements