*/translate_test: Do not pre-compute compressed data#713
*/translate_test: Do not pre-compute compressed data#713angelcerveraroldan wants to merge 2 commits into
Conversation
Unit tests that use pre-computed compressed data will fail in GoTip as the output of the compression function has changed. More importantly, the output is not guaranteed to be stable, so tests should now generate the zipped data at test-time, and we pre-define the expected input string.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new helper function CompressDataURL in base/util/test.go to dynamically generate compressed data URIs, and refactors several translation tests to use this helper instead of hardcoded base64 strings. The review feedback correctly identifies a potential nil pointer dereference when dereferencing the compression pointer returned by MakeDataURL in the new helper function.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| uri, compression, err := MakeDataURL(contents, nil, true) | ||
| if err != nil { | ||
| t.Fatalf("MakeDataURL: %v", err) | ||
| } | ||
| return uri, *compression |
There was a problem hiding this comment.
If MakeDataURL returns a nil pointer for compression (for example, if no compression is applied or in certain error/edge cases), dereferencing it directly with *compression will cause a runtime panic. It is safer to check if compression is nil before dereferencing it.
uri, compression, err := MakeDataURL(contents, nil, true)
if err != nil {
t.Fatalf("MakeDataURL: %v", err)
}
var comp string
if compression != nil {
comp = *compression
}
return uri, comp
|
Can we confirm that the new compressed format is understood by the old versions of Butane? I don't know fully grasp the impact this would have on the generated configs. |
The exact byte output of compressing functions may not be stable across different versions, so hard coding the expected byte output of the compression function will make tests fail or pass depending on the version of Go used to run the test.
angelcerveraroldan
force-pushed
the
fix-compression-tests
branch
from
b224bfa to
e655cc1
Compare
I think that Butane does not do any decompression, so there should be no issues here. Would the issue be "I generate an Ignition file using the new version of butane (compiled with Go 1.27 or newer), and try to decompress it with the old version of Ignition (compiled with Go 1.26 or older)" ? As far as I understand, there should be no issues with this. I compressed some strings with Gotip, and Go was able to decompress them just fine. The same was also true the other way around. The issue would come from anywhere that expects the gzipped data to remain constant across different version. |
100%
Yeah I think this is the worry, basically taking the compressed bits, and making sure when old compiled version of ignition creates the file from the compressed bits the output is == input. |
3 participants
Unit tests that use pre-computed compressed data will fail in GoTip as
the output of the compression function has changed.
More importantly, the output is not guaranteed to be stable, so tests
should now generate the zipped data at test-time, and we pre-define
the expected input string.
Fixes: #709