test(rain): add mysql integration coverage#17
Conversation
Greptile SummaryThis PR adds MySQL-backed integration coverage for Key observations:
Confidence Score: 5/5Safe to merge — the only finding is a P2 style suggestion (missing ParseTime in the programmatic DSN builder) that does not affect test correctness. All findings are P2 (non-blocking style/improvement suggestions). The test logic, DDL generation, cleanup ordering, and driver wiring are all correct. No data-integrity or correctness issues were identified. No files require special attention. Important Files Changed
|
| cfg := mysql.Config{ | ||
| Net: "tcp", | ||
| Addr: net.JoinHostPort(host, port), | ||
| User: user, | ||
| Passwd: strings.TrimSpace(os.Getenv("RAIN_MYSQL_PASSWORD")), | ||
| DBName: dbName, | ||
| } | ||
|
|
||
| return cfg.FormatDSN(), true |
There was a problem hiding this comment.
ParseTime not set in programmatic DSN builder
When constructing the DSN from individual env vars (RAIN_MYSQL_HOST / RAIN_MYSQL_USER / RAIN_MYSQL_DB), mysql.Config.ParseTime defaults to false. With this setting the MySQL driver returns DATETIME/TIMESTAMP column values as []byte (a string representation) rather than time.Time.
The test sidesteps a direct failure by declaring mysqlUserRow.CreatedAt as string instead of time.Time, but the omission is a footgun: anyone who copies this env-var path and expects TimestampTZ / time.Time columns to round-trip as proper Go time values will hit a runtime scan error. Adding ParseTime: true to the config struct is the conventional fix and matches what most ORM users would expect from an integration helper.
Prompt To Fix With AI
This is a comment left during a code review.
Path: pkg/rain/mysql_integration_test.go
Line: 164-172
Comment:
**`ParseTime` not set in programmatic DSN builder**
When constructing the DSN from individual env vars (`RAIN_MYSQL_HOST` / `RAIN_MYSQL_USER` / `RAIN_MYSQL_DB`), `mysql.Config.ParseTime` defaults to `false`. With this setting the MySQL driver returns `DATETIME`/`TIMESTAMP` column values as `[]byte` (a string representation) rather than `time.Time`.
The test sidesteps a direct failure by declaring `mysqlUserRow.CreatedAt` as `string` instead of `time.Time`, but the omission is a footgun: anyone who copies this env-var path and expects `TimestampTZ` / `time.Time` columns to round-trip as proper Go time values will hit a runtime scan error. Adding `ParseTime: true` to the config struct is the conventional fix and matches what most ORM users would expect from an integration helper.
How can I resolve this? If you propose a fix, please make it concise.
Motivation
pkg/rainruntime paths so CI and local devs can verify behavior against a real MySQL server.Description
pkg/rain/mysql_integration_test.gothat defines simpleusers/poststable models and tests table creation, inserts (defaults and explicit overrides), selects/scans, and an aliased join path.mysqlIntegrationDSN()supportingRAIN_MYSQL_DSNorRAIN_MYSQL_HOST/RAIN_MYSQL_USER/RAIN_MYSQL_DB(and optionalRAIN_MYSQL_PORTandRAIN_MYSQL_PASSWORD), and skip tests cleanly when configuration is missing.github.com/go-sql-driver/mysqltogo.mod/go.sumand import it in the test.README.mdto document the local command and environment variables to run the MySQL integration test (RAIN_MYSQL_DSN=... go test -race ./pkg/rain -run "^TestMySQLIntegration").Testing
make fmtwhich completed successfully.make lintwhich reported0 issues.make testwhich executed the test suite successfully (okfor packages and coverage reports generated).t.Skipwhen MySQL env vars are not provided; the test was not run against a live MySQL instance in this environment.Codex Task