Skip to content

Commit f93994a

Browse files
coryancoryan
authored
fix(sidekick): skip bumps for new crates (#2169)
Renamed `manifestVersionUpdated` to better capture what it does, fix the code to skip new files, and fixed all the tests.
1 parent 2d44b61 commit f93994a

File tree

2 files changed

+48
-18
lines changed

2 files changed

+48
-18
lines changed

‎internal/sidekick/internal/rust_release/update_manifest.go‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,13 @@ type cargo struct {
3838
}
3939

4040
func updateManifest(config *config.Release, lastTag, manifest string) ([]string, error) {
41-
updated, err := manifestVersionUpdated(config, lastTag, manifest)
41+
needsBump, err := manifestVersionNeedsBump(config, lastTag, manifest)
4242
if err != nil {
4343
return nil, err
4444
}
45+
if !needsBump {
46+
return nil, nil
47+
}
4548
contents, err := os.ReadFile(manifest)
4649
if err != nil {
4750
return nil, err
@@ -57,9 +60,6 @@ func updateManifest(config *config.Release, lastTag, manifest string) ([]string,
5760
if !info.Package.Publish {
5861
return nil, nil
5962
}
60-
if updated {
61-
return []string{info.Package.Name}, nil
62-
}
6363
newVersion, err := bumpPackageVersion(info.Package.Version)
6464
if err != nil {
6565
return nil, err
@@ -95,18 +95,20 @@ func bumpPackageVersion(version string) (string, error) {
9595
return strings.Join(components, "."), nil
9696
}
9797

98-
func manifestVersionUpdated(config *config.Release, lastTag, manifest string) (bool, error) {
98+
func manifestVersionNeedsBump(config *config.Release, lastTag, manifest string) (bool, error) {
9999
delta := fmt.Sprintf("%s..HEAD", lastTag)
100100
cmd := exec.Command(gitExe(config), "diff", delta, "--", manifest)
101101
cmd.Dir = "."
102102
contents, err := cmd.CombinedOutput()
103103
if err != nil {
104104
return false, err
105105
}
106+
if len(contents) == 0 {
107+
return true, nil
108+
}
106109
lines := strings.Split(string(contents), "\n")
107110
has := func(prefix string) bool {
108111
return slices.ContainsFunc(lines, func(line string) bool { return strings.HasPrefix(line, prefix) })
109112
}
110-
updated := has("+version") && has("-version")
111-
return updated, nil
113+
return !has("+version "), nil
112114
}

‎internal/sidekick/internal/rust_release/update_manifest_test.go‎

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ func TestUpdateManifestSuccess(t *testing.T) {
6363
if err != nil {
6464
t.Fatal(err)
6565
}
66+
want = nil
6667
if diff := cmp.Diff(want, got); diff != "" {
6768
t.Errorf("mismatch (-want, +got):\n%s", diff)
6869
}
@@ -227,7 +228,7 @@ func TestUpdateManifestBadSidekickConfig(t *testing.T) {
227228
}
228229

229230
if got, err := updateManifest(&release, tag, name); err == nil {
230-
t.Errorf("expected an error when using a bad version, got=%v", got)
231+
t.Errorf("expected an error when using a bad sidekick file, got=%v", got)
231232
}
232233
}
233234

@@ -252,7 +253,7 @@ func TestBumpPackageVersion(t *testing.T) {
252253
}
253254
}
254255

255-
func TestManifestVersionUpdatedSuccess(t *testing.T) {
256+
func TestManifestVersionNeedsBumpSuccess(t *testing.T) {
256257
const tag = "manifest-version-update-success"
257258
requireCommand(t, "git")
258259
setupForVersionBump(t, tag)
@@ -279,16 +280,43 @@ func TestManifestVersionUpdatedSuccess(t *testing.T) {
279280
t.Fatal(err)
280281
}
281282

282-
updated, err := manifestVersionUpdated(&release, tag, name)
283+
needsBump, err := manifestVersionNeedsBump(&release, tag, name)
284+
if err != nil {
285+
t.Fatal(err)
286+
}
287+
if needsBump {
288+
t.Errorf("expected no need for a bump for %s", name)
289+
}
290+
}
291+
292+
func TestManifestVersionNeedsBumpNewCrate(t *testing.T) {
293+
const tag = "manifest-version-update-new-crate"
294+
requireCommand(t, "git")
295+
setupForVersionBump(t, tag)
296+
release := config.Release{
297+
Remote: "origin",
298+
Branch: "main",
299+
Preinstalled: map[string]string{},
300+
}
301+
addGeneratedCrate(t, path.Join("src", "new"), "google-cloud-new")
302+
if err := external.Run("git", "add", "."); err != nil {
303+
t.Fatal(err)
304+
}
305+
if err := external.Run("git", "commit", "-m", "new crate", "."); err != nil {
306+
t.Fatal(err)
307+
}
308+
name := path.Join("src", "new", "Cargo.toml")
309+
310+
needsBump, err := manifestVersionNeedsBump(&release, tag, name)
283311
if err != nil {
284312
t.Fatal(err)
285313
}
286-
if !updated {
287-
t.Errorf("expected a change for %s, got=%v", name, updated)
314+
if needsBump {
315+
t.Errorf("no changes for new crates")
288316
}
289317
}
290318

291-
func TestManifestVersionUpdatedNoChange(t *testing.T) {
319+
func TestManifestVersionNeedsBumpNoChange(t *testing.T) {
292320
const tag = "manifest-version-update-no-change"
293321
requireCommand(t, "git")
294322
setupForVersionBump(t, tag)
@@ -298,16 +326,16 @@ func TestManifestVersionUpdatedNoChange(t *testing.T) {
298326
Preinstalled: map[string]string{},
299327
}
300328
name := path.Join("src", "storage", "Cargo.toml")
301-
updated, err := manifestVersionUpdated(&release, tag, name)
329+
needsBump, err := manifestVersionNeedsBump(&release, tag, name)
302330
if err != nil {
303331
t.Fatal(err)
304332
}
305-
if updated {
306-
t.Errorf("expected no change for %s, got=%v", name, updated)
333+
if !needsBump {
334+
t.Errorf("expected no change for %s", name)
307335
}
308336
}
309337

310-
func TestManifestVersionUpdatedBadDiff(t *testing.T) {
338+
func TestManifestVersionNeedsBumpBadDiff(t *testing.T) {
311339
const tag = "manifest-version-update-success"
312340
requireCommand(t, "git")
313341
setupForVersionBump(t, tag)
@@ -317,7 +345,7 @@ func TestManifestVersionUpdatedBadDiff(t *testing.T) {
317345
Preinstalled: map[string]string{},
318346
}
319347
name := path.Join("src", "storage", "Cargo.toml")
320-
if updated, err := manifestVersionUpdated(&release, "not-a-valid-tag", name); err == nil {
348+
if updated, err := manifestVersionNeedsBump(&release, "not-a-valid-tag", name); err == nil {
321349
t.Errorf("expected an error with an valid tag, got=%v", updated)
322350
}
323351
}

0 commit comments

Comments
 (0)