Updated SQL modes in tests to match production. - #31445
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis change updates the MySQL test environment to use stricter SQL modes that match production MySQL 8.0 defaults, improving data integrity checks. It introduces a new SQL mode constant, updates test database connections to use this mode, and adjusts test data to comply with stricter validation, such as timestamp handling and string length constraints. Changes
Sequence Diagram(s)Not applicable for this change, as it focuses on test configuration and data validation rather than control flow or feature logic. Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. All modifications are directly related to enforcing stricter SQL modes and updating test data to comply with these modes, as required by the linked issue. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #31445 +/- ##
=======================================
Coverage 63.73% 63.73%
=======================================
Files 1958 1958
Lines 190652 190705 +53
Branches 6179 6179
=======================================
+ Hits 121504 121550 +46
- Misses 59625 59627 +2
- Partials 9523 9528 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| } | ||
|
|
||
| // Check that RenewAt is not zero | ||
| if tok.RenewAt.IsZero() { |
There was a problem hiding this comment.
nit: Don't we want to do this check earlier?, so that we avoid doing the unnecessary encrypt call on this line
There was a problem hiding this comment.
Yes, makes sense. Thank you.
| `INSERT INTO nano_enrollments (id, device_id, type, topic, push_magic, token_hex, enabled) VALUES (?, ?, ?, ?, ?, ?, ?)`, | ||
| hostUUID, hostUUID, "Device", "topic", "push_magic", "token_hex", 1) | ||
| `INSERT INTO nano_enrollments (id, device_id, type, topic, push_magic, token_hex, enabled, last_seen_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, | ||
| hostUUID, hostUUID, "Device", "topic", "push_magic", "token_hex", 1, time.Now()) |
There was a problem hiding this comment.
Just double checking, does last_seen_at needs to be in UTC or doesn't matter? (I can't remember if the sqlx library auto-magically converts time.Time to UTC).
There was a problem hiding this comment.
The MySQL driver converts it to UTC for storage, so it doesn't matter. However, when retrieving it, it converts it back to the session timezone.
| title.Version, | ||
| title.ID, | ||
| "checksum", | ||
| hex.EncodeToString([]byte("checksum")), |
There was a problem hiding this comment.
Just curious, why do we have to hex encode the string just to UNHEX it while inserting the data?
There was a problem hiding this comment.
You're right. It is not needed here.
juan-fdz-hawa
left a comment
juan-fdz-hawa
left a comment
There was a problem hiding this comment.
LGTM just a couple of small questions.
|
@juan-fdz-hawa I will merge this PR and address your suggestions in a follow up PR. |
3 participants
Fixes #31444
The changes are primarily in tests. The only changes in production code are a couple validations/checks for invalid values in:
Checklist for submitter
If some of the following don't apply, delete the relevant line.
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Testing
Summary by CodeRabbit
Bug Fixes
Chores
Tests