Fixed broken tests#48613
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
👮 Files not reviewed due to content moderation or server errors (2)
Warning Walkthrough skippedFile diffs could not be summarized. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
juan-fdz-hawa
force-pushed
the
48467-fix-broken-tests
branch
from
370295e to
93f7098
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #48613 +/- ##
=======================================
Coverage 68.03% 68.03%
=======================================
Files 3678 3678
Lines 233789 233789
Branches 12454 12454
=======================================
+ Hits 159047 159051 +4
+ Misses 60434 60433 -1
+ Partials 14308 14305 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| for _, row := range rows[1:] { | ||
| if row[0] == fmt.Sprint(hosts[2].ID) { | ||
| require.Equal(t, "a@b.c,b@b.c", row[2], row) | ||
| // the order of the emails is not guaranteed |
There was a problem hiding this comment.
are the order of emails supposed to be guaranteed?
There was a problem hiding this comment.
Looks like we have some inconsistency in our device mapping API sorting:
Fleet has two code paths that turn the same host_emails rows into a device_mapping array, and today they
disagree:
Path A — sorted (ORDER BY email, source) via listHostDeviceMappingDB → ListHostDeviceMapping. This
serves:
- GET /hosts/{id}/device_mapping — the host details page (hosts.go:2240)
- PUT /hosts/{id}/device_mapping — returns the updated list after adding a custom email (hosts.go:2357)
- GET /device/{token}/device_mapping — the end-user "My Device" page (devices.go:420)
Path B — unordered (GROUP_CONCAT with no ORDER BY) via the list query. This serves:
- GET /hosts?device_mapping=true — the Hosts table (hosts.go:1311)
- GET /labels/{id}/hosts?device_mapping=true — a label's host list (labels.go:1237)
- GET /hosts/report — the CSV export (same query)
There was a problem hiding this comment.
Maybe ok to merge this fix as is and file a new bug to fix the inconsistencies, TMWYT
There was a problem hiding this comment.
my guess is that users don't care about the ordering (no bug reported yet), so that's why it made sense to fix this at the test layer instead of changing production code ATM.
2 participants
Uh oh!
There was an error while loading. Please reload this page.