You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "123123213weqw (via GitHub)" <gi...@apache.org> on 2026/10/12 11:27:55 UTC

[PR] test(rmqctl): pin the YAML number round-trip in the existing test [rocketmq-dashboard]

123123213weqw opened a new pull request, #6371:
URL: https://github.com/apache/rocketmq-dashboard/pull/6371

   Supersedes #6136, folded per the salvage path in the review there (reopening the old PR was refused by GitHub, hence the new one).
   
   ## What
   
   `TestYAMLRestoresJSONNumbersTest` in `rmqctl/internal/output/output_test.go` already pins that `json.Number` leaves render unquoted in YAML and keep exact digits beyond 2^53. What it did not observe was the other end of the contract: a consumer decoding the rendered YAML must read the types back — integers as integers (never bent through a float64), floats as floats.
   
   ## The change
   
   - The round-trip decode assertions from #6136 now live inside the existing test instead of a second, near-duplicate file: `yaml.Unmarshal` on the rendered output, then `skippedCount` as `int 5`, `size` as `int 3`, `ratio` as `float64 0.25`, and `offset` accepted as `int`/`int64`/`uint64` — always exactly `9007199254740993` (2^53+1). Dropping `restoreNumbers` still trips the render assertions; re-introducing a float64 anywhere in the path now trips the decode ones too.
   - The near-duplicate `yaml_numbers_test.go` from #6136 is deleted; one test owns the whole contract.
   - Blank line between the ASF license header and `package output`, matching the neighbouring files.
   - Branch also merged onto the current `rocketmq-studio` (no conflicts); `go test ./internal/output/` is green.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] test(rmqctl): pin the YAML number round-trip in the existing test [rocketmq-dashboard]

Posted by "lizhimins (via GitHub)" <gi...@apache.org>.
lizhimins commented on PR #6371:
URL: https://github.com/apache/rocketmq-dashboard/pull/6371#issuecomment-6190300258

   Closing in favour of #6439, which is now merged into `rocketmq-studio` as `eeaa90b91`. Thanks for folding this into the existing test instead of a second file — that part of the #6136 salvage path is exactly right, and the ASF-header blank line matches the neighbours. Closing this one anyway, because the added decode block does not increase coverage: `output_test.go:45` already pins the rendered text of all four scalars (`size: 3`, `ratio: 0.25`, `skippedCount: 5`, `offset: 9007199254740993`) and `:50` pins that none of them is quoted, and `yaml.Unmarshal` is a deterministic function of that text. Any mutation the decode block could catch (e.g. routing an integer through float64) changes the rendered literal first, so the render assertions already go red — as your own note concedes. The `int`/`int64`/`uint64` type switch also asserts a property of yaml.v3 rather than of `restoreNumbers` (`output.go:61-70`). Salvage path if you want decode-side coverage to be worth carrying: as
 sert on values whose rendered text is *not* already pinned — that is precisely what #6439 and #6490 do for literals outside int64/float64 range (`18446744073709551615`, `-9223372036854775809`, `1e400`, and the `!!int` tag yaml.v3 emits for the one value it cannot resolve back into a Go int). Folding a decode assertion into either of those tests would give the round-trip contract a mutation-sensitive owner; this file's existing test already owns the render half.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] test(rmqctl): pin the YAML number round-trip in the existing test [rocketmq-dashboard]

Posted by "lizhimins (via GitHub)" <gi...@apache.org>.
lizhimins closed pull request #6371: test(rmqctl): pin the YAML number round-trip in the existing test
URL: https://github.com/apache/rocketmq-dashboard/pull/6371


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org