Merge the weather status check (#236)
CurrentWeather and geocodeOne decoded the body without checking the status, so a non-200 became a successful zero-value answer. He was told it is 0 degrees, or that the city he named does not exist. The second blamed him for a service failure. Both now check the status and return an error naming it. The caller needed no change: it already branches on not-configured, unknown-location and a generic error in that order. Three httptest cases cover what nothing covered before. A 500 and a connection refused still produce one sentence, and the agent said so rather than rounding it up. Splitting them was not asked for and both are an honest named gap. (V-589)
This commit is contained in:
@@ -83,6 +83,9 @@ func (p *OpenMeteoProvider) CurrentWeather(ctx context.Context, location string)
|
||||
return Weather{}, fmt.Errorf("open-meteo: http: %w", err)
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return Weather{}, fmt.Errorf("open-meteo forecast: http %d", resp.StatusCode)
|
||||
}
|
||||
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
if err != nil {
|
||||
@@ -192,6 +195,9 @@ func (p *OpenMeteoProvider) geocodeOne(ctx context.Context, location string) (la
|
||||
return 0, 0, "", err
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return 0, 0, "", fmt.Errorf("open-meteo geocode: http %d", resp.StatusCode)
|
||||
}
|
||||
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
if err != nil {
|
||||
|
||||
@@ -3,9 +3,11 @@ package weather
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
@@ -76,6 +78,87 @@ func (t *mockTransport) RoundTrip(r *http.Request) (*http.Response, error) {
|
||||
return http.DefaultTransport.RoundTrip(req)
|
||||
}
|
||||
|
||||
// TestOpenMeteoProvider_ForecastHTTPError — a non-200 forecast reply must not
|
||||
// decode into a zero-value Weather{Temperature: 0, Condition: "неизвестно"}
|
||||
// reported as success (Vikunja #589). It must be a distinct error, not
|
||||
// ErrLocationUnknown, so the caller does not blame the owner's city.
|
||||
func TestOpenMeteoProvider_ForecastHTTPError(t *testing.T) {
|
||||
mock := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.URL.Path == "/v1/search" {
|
||||
json.NewEncoder(w).Encode(geoResponse{
|
||||
Results: []geoResult{{Name: "Moscow", Latitude: 55.7558, Longitude: 37.6173, Country: "Russia"}},
|
||||
})
|
||||
return
|
||||
}
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
}))
|
||||
defer mock.Close()
|
||||
|
||||
p := NewOpenMeteoProvider()
|
||||
p.SetClient(&http.Client{Transport: &mockTransport{
|
||||
geoURL: mock.URL + "/v1/search",
|
||||
forecastURL: mock.URL + "/v1/forecast",
|
||||
}})
|
||||
|
||||
w, err := p.CurrentWeather(context.Background(), "Moscow")
|
||||
if err == nil {
|
||||
t.Fatalf("CurrentWeather: want error, got Weather{%+v}", w)
|
||||
}
|
||||
if errors.Is(err, ErrLocationUnknown) {
|
||||
t.Fatalf("CurrentWeather: want a service error, got ErrLocationUnknown: %v", err)
|
||||
}
|
||||
if !strings.Contains(err.Error(), "500") {
|
||||
t.Errorf("CurrentWeather: error %q does not name the status", err.Error())
|
||||
}
|
||||
}
|
||||
|
||||
// TestOpenMeteoProvider_GeocodeHTTPError — a non-200 geocode reply must not be
|
||||
// read as "no such city" (ErrLocationUnknown). A 500 is a service outage, and
|
||||
// the owner hears a different sentence for each (Vikunja #589).
|
||||
func TestOpenMeteoProvider_GeocodeHTTPError(t *testing.T) {
|
||||
mock := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
}))
|
||||
defer mock.Close()
|
||||
|
||||
p := NewOpenMeteoProvider()
|
||||
p.SetClient(&http.Client{Transport: &mockTransport{
|
||||
geoURL: mock.URL + "/v1/search",
|
||||
forecastURL: mock.URL + "/v1/forecast",
|
||||
}})
|
||||
|
||||
_, err := p.CurrentWeather(context.Background(), "Уфе")
|
||||
if err == nil {
|
||||
t.Fatal("CurrentWeather: want error")
|
||||
}
|
||||
if errors.Is(err, ErrLocationUnknown) {
|
||||
t.Fatalf("CurrentWeather: want a service error, got ErrLocationUnknown: %v", err)
|
||||
}
|
||||
if !strings.Contains(err.Error(), "500") {
|
||||
t.Errorf("CurrentWeather: error %q does not name the status", err.Error())
|
||||
}
|
||||
}
|
||||
|
||||
// TestOpenMeteoProvider_GeocodeEmptyResult — a genuine 200 reply with no
|
||||
// results is still ErrLocationUnknown, distinct from a service failure.
|
||||
func TestOpenMeteoProvider_GeocodeEmptyResult(t *testing.T) {
|
||||
mock := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
json.NewEncoder(w).Encode(geoResponse{Results: nil})
|
||||
}))
|
||||
defer mock.Close()
|
||||
|
||||
p := NewOpenMeteoProvider()
|
||||
p.SetClient(&http.Client{Transport: &mockTransport{
|
||||
geoURL: mock.URL + "/v1/search",
|
||||
forecastURL: mock.URL + "/v1/forecast",
|
||||
}})
|
||||
|
||||
_, err := p.CurrentWeather(context.Background(), "Атлантида")
|
||||
if !errors.Is(err, ErrLocationUnknown) {
|
||||
t.Fatalf("CurrentWeather: want ErrLocationUnknown, got %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestStubProvider(t *testing.T) {
|
||||
p := NewStubProvider()
|
||||
_, err := p.CurrentWeather(context.Background(), "Moscow")
|
||||
|
||||
Reference in New Issue
Block a user