-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fixed tariff: fix sorting zones with deviating months or days #26425
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
Changes from 8 commits
e276eed
7ebe633
b026142
8ce22dd
777e462
04a1914
974c992
4182e81
515e502
59bb8a0
3794bef
341f04a
14c6c52
004a5fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -78,5 +78,27 @@ HOURS: | |
| res = append(res, HourMin{Hour: hour, Min: 0}) | ||
| } | ||
|
|
||
| // Sort markers by time to ensure correct ordering | ||
| slices.SortFunc(res, func(a, b HourMin) int { | ||
| return a.Minutes() - b.Minutes() | ||
| }) | ||
|
|
||
|
Comment on lines
+81
to
+85
Contributor
Author
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. major required fix - paired with correcting rates clock |
||
| return res | ||
| } | ||
|
|
||
| // MoreSpecific returns true if zone a is more specific than zone b. | ||
| // A zone is more specific if it has constraints on fewer days or months. | ||
| func MoreSpecific(a, b Zone) bool { | ||
iseeberg79 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return specificity(a) > specificity(b) | ||
| } | ||
|
|
||
| func specificity(z Zone) int { | ||
| spec := 0 | ||
| if len(z.Days) > 0 && len(z.Days) < 7 { | ||
| spec++ | ||
| } | ||
| if len(z.Months) > 0 && len(z.Months) < 12 { | ||
| spec++ | ||
| } | ||
| return spec | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| package tariff | ||
|
|
||
| import ( | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/benbjohnson/clock" | ||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| // TestFixedSpecificity validates that MoreSpecific logic is necessary. | ||
| // Edge case: zones with IDENTICAL hours but different month/day constraints. | ||
| // Without MoreSpecific, the result would depend on undefined sort order. | ||
| func TestFixedSpecificity(t *testing.T) { | ||
| at, err := NewFixedFromConfig(map[string]any{ | ||
| "price": 0.30, | ||
| "zones": []struct { | ||
| Price float64 | ||
| Hours string | ||
| Months string | ||
| }{ | ||
| // REVERSED: specific first, general second | ||
| // Without MoreSpecific, this will fail! | ||
| {0.10, "0-5", "Jan-Mar,Oct-Dec"}, // specific (winter only) | ||
|
Member
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. Am Ende müssen vmtl. auch Überlappungen gehandhabt werden? In jedem Fall wäre es schön diese- wenn nicht vergleichbar- als Fehler identifizieren zu können.
Contributor
Author
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. aber nur wenn wirklich "ambigious"? Das braucht ein paar Helper |
||
| {0.20, "0-5", ""}, // general (all year) | ||
| }, | ||
| }) | ||
| require.NoError(t, err) | ||
|
|
||
| testCases := []struct { | ||
| month time.Month | ||
| expected float64 | ||
| }{ | ||
| {time.January, 0.10}, // winter: specific zone wins | ||
| {time.June, 0.20}, // summer: general zone | ||
| {time.December, 0.10}, // winter: specific zone wins | ||
| } | ||
|
|
||
| // Test both UTC and Local timezones | ||
| timezones := []*time.Location{time.UTC, time.Local} | ||
| for _, tz := range timezones { | ||
| t.Run(tz.String(), func(t *testing.T) { | ||
| for _, tc := range testCases { | ||
| clock := clock.NewMock() | ||
| at.(*Fixed).clock = clock | ||
| clock.Set(time.Date(2025, tc.month, 15, 3, 0, 0, 0, tz)) | ||
|
|
||
| rr, err := at.Rates() | ||
| require.NoError(t, err) | ||
|
|
||
| r, err := rr.At(clock.Now()) | ||
| require.NoError(t, err) | ||
|
|
||
| assert.Equal(t, tc.expected, r.Value, | ||
| "TZ=%s, %s: expected %.2f", tz, tc.month, tc.expected) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
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.
required, fix clock dependencies