Skip to content

Commit a4a5e71

Browse files
committed
feedback
1 parent 00142c6 commit a4a5e71

File tree

4 files changed

+49
-17
lines changed

4 files changed

+49
-17
lines changed

internal/app/azldev/cmds/component/diffidentity.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,13 @@ func readIdentityFile(
130130
// ComputeDiff compares base and head identity maps and produces a diff report.
131131
// When changedOnly is true, the Removed and Unchanged lists are left empty.
132132
func ComputeDiff(base map[string]string, head map[string]string, changedOnly bool) *IdentityDiffReport {
133-
report := &IdentityDiffReport{}
133+
// Initialize all slices so JSON serialization produces [] instead of null.
134+
report := &IdentityDiffReport{
135+
Changed: make([]string, 0),
136+
Added: make([]string, 0),
137+
Removed: make([]string, 0),
138+
Unchanged: make([]string, 0),
139+
}
134140

135141
// Check base components against head.
136142
for name, baseFP := range base {

internal/app/azldev/cmds/component/diffidentity_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@
44
package component_test
55

66
import (
7+
"encoding/json"
78
"testing"
89

10+
"github.com/microsoft/azure-linux-dev-tools/internal/app/azldev"
911
"github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/cmds/component"
1012
"github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/testutils"
1113
"github.com/microsoft/azure-linux-dev-tools/internal/utils/fileperms"
@@ -200,3 +202,34 @@ func TestDiffIdentities_EmptyArray(t *testing.T) {
200202
require.True(t, ok)
201203
assert.Empty(t, tableResults)
202204
}
205+
206+
func TestDiffIdentities_JSONFormat(t *testing.T) {
207+
testEnv := testutils.NewTestEnv(t)
208+
testEnv.Env.SetDefaultReportFormat(azldev.ReportFormatJSON)
209+
210+
require.NoError(t, fileutils.WriteFile(testEnv.TestFS, "/base.json",
211+
[]byte(`[{"component":"curl","fingerprint":"sha256:aaa"}]`), fileperms.PublicFile))
212+
require.NoError(t, fileutils.WriteFile(testEnv.TestFS, "/head.json",
213+
[]byte(`[{"component":"curl","fingerprint":"sha256:bbb"},{"component":"wget","fingerprint":"sha256:ccc"}]`),
214+
fileperms.PublicFile))
215+
216+
result, err := component.DiffIdentities(testEnv.Env, "/base.json", "/head.json", false)
217+
require.NoError(t, err)
218+
219+
report, ok := result.(*component.IdentityDiffReport)
220+
require.True(t, ok, "expected IdentityDiffReport for JSON format")
221+
222+
assert.Equal(t, []string{"curl"}, report.Changed)
223+
assert.Equal(t, []string{"wget"}, report.Added)
224+
assert.Empty(t, report.Removed)
225+
assert.Empty(t, report.Unchanged)
226+
227+
// Verify JSON serialization produces [] not null for empty arrays.
228+
jsonBytes, err := json.Marshal(report)
229+
require.NoError(t, err)
230+
231+
jsonStr := string(jsonBytes)
232+
assert.Contains(t, jsonStr, `"removed":[]`)
233+
assert.Contains(t, jsonStr, `"unchanged":[]`)
234+
assert.NotContains(t, jsonStr, "null")
235+
}

internal/app/azldev/cmds/component/identity.go

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ func ComputeComponentIdentities(
9191
distroRef := env.Config().Project.DefaultDistro
9292

9393
// Resolve the distro definition (fills in default version for the fingerprint).
94-
distroRef, _, err = resolveDistroForIdentity(env, distroRef)
94+
distroRef, err = resolveDistroForIdentity(env, distroRef)
9595
if err != nil {
9696
slog.Debug("Could not resolve distro", "error", err)
9797
}
@@ -214,14 +214,14 @@ func computeSingleIdentity(
214214
}, nil
215215
}
216216

217-
// resolveDistroForIdentity resolves the default distro reference and returns a
218-
// [sourceproviders.ResolvedDistro] for constructing a source manager.
217+
// resolveDistroForIdentity resolves the default distro reference, filling in the
218+
// default version when unspecified.
219219
func resolveDistroForIdentity(
220220
env *azldev.Env, distroRef projectconfig.DistroReference,
221-
) (projectconfig.DistroReference, sourceproviders.ResolvedDistro, error) {
222-
distroDef, distroVersionDef, err := env.ResolveDistroRef(distroRef)
221+
) (projectconfig.DistroReference, error) {
222+
distroDef, _, err := env.ResolveDistroRef(distroRef)
223223
if err != nil {
224-
return distroRef, sourceproviders.ResolvedDistro{},
224+
return distroRef,
225225
fmt.Errorf("resolving distro %#q:\n%w", distroRef.Name, err)
226226
}
227227

@@ -230,11 +230,7 @@ func resolveDistroForIdentity(
230230
distroRef.Version = distroDef.DefaultVersion
231231
}
232232

233-
return distroRef, sourceproviders.ResolvedDistro{
234-
Ref: distroRef,
235-
Definition: distroDef,
236-
Version: distroVersionDef,
237-
}, nil
233+
return distroRef, nil
238234
}
239235

240236
// countAffectsCommits counts the number of "Affects: <componentName>" commits in the
@@ -281,6 +277,8 @@ func resolveSourceIdentityForComponent(
281277
}
282278

283279
// Upstream components: need a source manager with the component's resolved distro.
280+
// A new SourceManager is created per component because each component can reference
281+
// a different upstream distro, making shared caching impractical.
284282
distro, err := sourceproviders.ResolveDistro(env, comp)
285283
if err != nil {
286284
return "", fmt.Errorf("resolving distro for component %#q:\n%w",

internal/utils/git/git.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -173,11 +173,6 @@ func (g *GitProviderImpl) LsRemoteHead(
173173
return "", errors.New("repository URL cannot be empty")
174174
}
175175

176-
_, err := url.Parse(repoURL)
177-
if err != nil {
178-
return "", fmt.Errorf("invalid URL %#q:\n%w", repoURL, err)
179-
}
180-
181176
if branch == "" {
182177
return "", errors.New("branch cannot be empty")
183178
}

0 commit comments

Comments
 (0)