diff --git a/go.mod b/go.mod index 11588d174..3c127d6e8 100644 --- a/go.mod +++ b/go.mod @@ -282,6 +282,8 @@ replace github.com/docker/docker => github.com/docker/docker v27.5.1+incompatibl replace github.com/gfleury/go-bitbucket-v1 => github.com/gfleury/go-bitbucket-v1 v0.0.0-20230825095122-9bc1711434ab +// replace github.com/jfrog/build-info-go => github.com/jfrog/build-info-go v1.8.9-0.20251118093509-ebe86db606fa + //replace github.com/jfrog/jfrog-cli-artifactory => github.com/naveenku-jfrog/jfrog-cli-artifactory v0.0.0-20251210184507-0c8d138690cb //replace github.com/jfrog/build-info-go => github.com/jfrog/build-info-go v1.8.9-0.20251006061821-8b1be6a65215 diff --git a/nuget_test.go b/nuget_test.go index f0440fb41..d9f4708e7 100644 --- a/nuget_test.go +++ b/nuget_test.go @@ -3,9 +3,6 @@ package main import ( "encoding/xml" "fmt" - "github.com/jfrog/jfrog-cli-core/v2/utils/ioutils" - "github.com/jfrog/jfrog-client-go/http/httpclient" - "github.com/jfrog/jfrog-client-go/utils/io" "net/http" "os" "os/exec" @@ -14,6 +11,10 @@ import ( "strings" "testing" + "github.com/jfrog/jfrog-cli-core/v2/utils/ioutils" + "github.com/jfrog/jfrog-client-go/http/httpclient" + "github.com/jfrog/jfrog-client-go/utils/io" + dotnetUtils "github.com/jfrog/build-info-go/build/utils/dotnet" buildInfo "github.com/jfrog/build-info-go/entities" biutils "github.com/jfrog/build-info-go/utils" @@ -163,9 +164,8 @@ func testNugetCmd(t *testing.T, projectPath, buildName, buildNumber string, expe } // Add allow insecure connection for testings to work with localhost server -func allowInsecureConnectionForTests(args *[]string) *[]string { +func allowInsecureConnectionForTests(args *[]string) { *args = append(*args, "--allow-insecure-connections") - return args } func assertNugetDependencies(t *testing.T, module buildInfo.Module, moduleName string) { @@ -375,3 +375,102 @@ func prepareSetupTest(t *testing.T, packageManager project.ProjectType) func() { restoreDir() } } + +// TestDotnetRequestedByDeterminism verifies that the RequestedBy paths in build-info +// are deterministic across multiple runs. This addresses the bug where dependencies +// could flip between "direct" and "transitive" attribution due to non-deterministic +// map iteration order in Go. +// +// The test uses the "determinism" project which has dependencies that are BOTH +// direct AND transitive (e.g., Newtonsoft.Json is a direct dep and also a transitive +// dep of NuGet.Core). This creates multiple RequestedBy paths that must be +// consistently sorted across runs. +func TestDotnetRequestedByDeterminism(t *testing.T) { + initNugetTest(t) + const numRuns = 5 + buildName := tests.DotnetBuildName + "-determinism" + + // Use "determinism" project which has deps that are both direct AND transitive + projectPath := createNugetProject(t, "determinism") + err := createConfigFileForTest([]string{projectPath}, tests.NugetRemoteRepo, "", t, project.Dotnet, false) + require.NoError(t, err) + + wd, err := os.Getwd() + require.NoError(t, err) + chdirCallback := clientTestUtils.ChangeDirWithCallback(t, wd, projectPath) + defer chdirCallback() + + // Store RequestedBy maps from each run for comparison + var allRunsRequestedBy []map[string][][]string + + for i := 1; i <= numRuns; i++ { + buildNumber := strconv.Itoa(i) + args := []string{dotnetUtils.DotnetCore.String(), "restore"} + allowInsecureConnectionForTests(&args) + args = append(args, "--build-name="+buildName, "--build-number="+buildNumber) + + err := runNuGet(t, args...) + require.NoError(t, err, "Run %d failed", i) + + // Publish build info + require.NoError(t, artifactoryCli.Exec("bp", buildName, buildNumber)) + + // Get published build info + publishedBuildInfo, found, err := tests.GetBuildInfo(serverDetails, buildName, buildNumber) + require.NoError(t, err) + require.True(t, found, "Build info not found for run %d", i) + + bi := publishedBuildInfo.BuildInfo + require.NotEmpty(t, bi.Modules, "No modules in build info for run %d", i) + + // Extract RequestedBy for each dependency + requestedByMap := make(map[string][][]string) + for _, module := range bi.Modules { + for _, dep := range module.Dependencies { + requestedByMap[dep.Id] = dep.RequestedBy + } + } + allRunsRequestedBy = append(allRunsRequestedBy, requestedByMap) + + // Clean up this build number + inttestutils.DeleteBuild(serverDetails.ArtifactoryUrl, buildName, artHttpDetails) + } + + // Compare all runs - they should be identical + firstRun := allRunsRequestedBy[0] + for i := 1; i < numRuns; i++ { + currentRun := allRunsRequestedBy[i] + + // Check that all dependencies have the same RequestedBy paths + for depId, expectedRequestedBy := range firstRun { + actualRequestedBy, exists := currentRun[depId] + assert.True(t, exists, "Dependency %s missing in run %d", depId, i+1) + assert.Equal(t, expectedRequestedBy, actualRequestedBy, + "RequestedBy mismatch for %s between run 1 and run %d.\nExpected: %v\nActual: %v", + depId, i+1, expectedRequestedBy, actualRequestedBy) + } + + // Also check for any extra dependencies in current run + for depId := range currentRun { + _, exists := firstRun[depId] + assert.True(t, exists, "Extra dependency %s found in run %d but not in run 1", depId, i+1) + } + } + + // Verify that dependencies with multiple paths have direct path first + // Newtonsoft.Json should have direct path ["determinism"] before transitive paths + for depId, requestedBy := range firstRun { + if len(requestedBy) > 1 { + // Direct paths (length 1) should come before transitive paths (length > 1) + for j := 1; j < len(requestedBy); j++ { + assert.LessOrEqual(t, len(requestedBy[j-1]), len(requestedBy[j]), + "RequestedBy paths not sorted by length for %s: path %d has length %d, path %d has length %d", + depId, j-1, len(requestedBy[j-1]), j, len(requestedBy[j])) + } + t.Logf("Dependency %s has %d RequestedBy paths (verified sorted)", depId, len(requestedBy)) + } + } + + t.Logf("Successfully verified RequestedBy determinism across %d runs", numRuns) + cleanTestsHomeEnv() +} diff --git a/testdata/nuget/determinism/Program.cs b/testdata/nuget/determinism/Program.cs new file mode 100644 index 000000000..190247ca7 --- /dev/null +++ b/testdata/nuget/determinism/Program.cs @@ -0,0 +1,4 @@ +// Test project for RequestedBy determinism fix +// See determinism.csproj for dependency setup details +Console.WriteLine("Determinism test project"); + diff --git a/testdata/nuget/determinism/determinism.csproj b/testdata/nuget/determinism/determinism.csproj new file mode 100644 index 000000000..df6cc4e24 --- /dev/null +++ b/testdata/nuget/determinism/determinism.csproj @@ -0,0 +1,38 @@ + + + + + + Exe + net6.0 + enable + enable + + + + + + + + + + + + + + + diff --git a/testdata/nuget/determinism/determinism.sln b/testdata/nuget/determinism/determinism.sln new file mode 100644 index 000000000..45b5bbde0 --- /dev/null +++ b/testdata/nuget/determinism/determinism.sln @@ -0,0 +1,19 @@ +Microsoft Visual Studio Solution File, Format Version 12.00 +# Visual Studio Version 17 +VisualStudioVersion = 17.0.31903.59 +MinimumVisualStudioVersion = 10.0.40219.1 +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "determinism", "determinism.csproj", "{D1FFA0DC-0ACC-4108-ADC1-2A71122C09AF}" +EndProject +Global + GlobalSection(SolutionConfigurationPlatforms) = preSolution + Debug|Any CPU = Debug|Any CPU + Release|Any CPU = Release|Any CPU + EndGlobalSection + GlobalSection(ProjectConfigurationPlatforms) = postSolution + {D1FFA0DC-0ACC-4108-ADC1-2A71122C09AF}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {D1FFA0DC-0ACC-4108-ADC1-2A71122C09AF}.Debug|Any CPU.Build.0 = Debug|Any CPU + {D1FFA0DC-0ACC-4108-ADC1-2A71122C09AF}.Release|Any CPU.ActiveCfg = Release|Any CPU + {D1FFA0DC-0ACC-4108-ADC1-2A71122C09AF}.Release|Any CPU.Build.0 = Release|Any CPU + EndGlobalSection +EndGlobal +