Skip to content

Fix Scan repository failing tests #1239

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
eranturgeman merged 14 commits into from
Mar 4, 2026
Merged

Fix Scan repository failing tests #1239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
1a87330
fix scan repository failing tests
eranturgeman Mar 3, 2026
ba03cc9
fix parallelism issue with windows runner
eranturgeman Mar 3, 2026
8ba132d
adding missed technology after merging its fixer
eranturgeman Mar 3, 2026
3f8f18e
.
eranturgeman Mar 3, 2026
498e4c5
attempt to fix windows runner issue
eranturgeman Mar 3, 2026
958b4b9
.
eranturgeman Mar 3, 2026
1c6fe86
another attempt to solve windows issue
eranturgeman Mar 3, 2026
6e51153
another attempt to solve windows issue
eranturgeman Mar 3, 2026
79b34c6
Merge branch 'v3_er' of https://github.com/jfrog/frogbot into fix-sca…
eranturgeman Mar 4, 2026
5cfa69e
another attempt to solve windows issue
eranturgeman Mar 4, 2026
6d1e4af
fix
eranturgeman Mar 4, 2026
955d5f6
fix Go-Sec errors
eranturgeman Mar 4, 2026
b62eb54
fix Go-Sec errors
eranturgeman Mar 4, 2026
3d55828
fix Go-Sec errors
eranturgeman Mar 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 69 additions & 57 deletions scanrepository/scanrepository_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"errors"
"fmt"
services2 "github.com/jfrog/jfrog-client-go/xsc/services"
"io"
"net/http"
"net/http/httptest"
"os"
Expand Down Expand Up @@ -42,23 +43,6 @@ import (

const rootTestDir = "scanrepository"

var emptyConfigProfile = services2.ConfigProfile{
ProfileName: "test-profile",
GeneralConfig: services2.GeneralConfig{},
FrogbotConfig: services2.FrogbotConfig{
BranchNameTemplate: "",
PrTitleTemplate: "",
CommitMessageTemplate: "",
},
Modules: []services2.Module{
{
ModuleId: 0,
ModuleName: "test-module",
PathFromRoot: ".",
},
},
}

var testPackagesData = []struct {
packageType string
commandName string
Expand Down Expand Up @@ -117,22 +101,23 @@ func TestScanRepositoryCmd_Run(t *testing.T) {
expectedMissingFilesInBranch map[string][]string
packageDescriptorPaths []string
aggregateFixes bool
allowPartialResults bool
failUponAnyScannerError bool
}{
{
testName: "aggregate",
expectedPackagesInBranch: map[string][]string{"frogbot-update-68d9dee2475e5986e783d85dfa11baa0-dependencies-master": {"uuid", "minimist", "mpath"}},
expectedVersionUpdatesInBranch: map[string][]string{"frogbot-update-68d9dee2475e5986e783d85dfa11baa0-dependencies-master": {"^1.2.6", "^9.0.0", "^0.8.4"}},
expectedVersionUpdatesInBranch: map[string][]string{"frogbot-update-68d9dee2475e5986e783d85dfa11baa0-dependencies-master": {"1.2.6", "^9.0.0", "0.8.4"}},
packageDescriptorPaths: []string{"package.json"},
aggregateFixes: true,
failUponAnyScannerError: true,
},
{
testName: "aggregate-multi-dir",
expectedPackagesInBranch: map[string][]string{"frogbot-update-68d9dee2475e5986e783d85dfa11baa0-dependencies-master": {"uuid", "minimatch", "mpath", "minimist"}},
expectedVersionUpdatesInBranch: map[string][]string{"frogbot-update-68d9dee2475e5986e783d85dfa11baa0-dependencies-master": {"^1.2.6", "^9.0.0", "^0.8.4", "^3.0.5"}},
expectedMissingFilesInBranch: map[string][]string{"frogbot-update-68d9dee2475e5986e783d85dfa11baa0-dependencies-master": {"npm1/package-lock.json", "npm2/package-lock.json"}},
expectedVersionUpdatesInBranch: map[string][]string{"frogbot-update-68d9dee2475e5986e783d85dfa11baa0-dependencies-master": {"1.2.6", "^9.0.0", "0.8.4"}},
packageDescriptorPaths: []string{"npm1/package.json", "npm2/package.json"},
aggregateFixes: true,
failUponAnyScannerError: true,
},
{
testName: "aggregate-no-vul",
Expand All @@ -141,22 +126,25 @@ func TestScanRepositoryCmd_Run(t *testing.T) {
expectedVersionUpdatesInBranch: map[string][]string{"master": {}},
packageDescriptorPaths: []string{"package.json"},
aggregateFixes: true,
failUponAnyScannerError: true,
},
{
testName: "aggregate-cant-fix",
// Branch name stays master as no new branch is being created
expectedPackagesInBranch: map[string][]string{"master": {}},
expectedVersionUpdatesInBranch: map[string][]string{"master": {}},
// This is a build tool dependency which should not be fixed.
packageDescriptorPaths: []string{"setup.py"},
aggregateFixes: true,
packageDescriptorPaths: []string{"setup.py"},
aggregateFixes: true,
failUponAnyScannerError: true,
},
{
testName: "non-aggregate",
expectedPackagesInBranch: map[string][]string{"frogbot-minimist-258ad6a538b5ba800f18ae4f6d660302": {"minimist"}},
expectedVersionUpdatesInBranch: map[string][]string{"frogbot-minimist-258ad6a538b5ba800f18ae4f6d660302": {"^1.2.6"}},
expectedVersionUpdatesInBranch: map[string][]string{"frogbot-minimist-258ad6a538b5ba800f18ae4f6d660302": {"1.2.6"}},
packageDescriptorPaths: []string{"package.json"},
aggregateFixes: false,
failUponAnyScannerError: true,
},
{
// This testcase checks the partial results feature. It simulates a failure in the dependency tree construction in the test's project inner module
Expand All @@ -165,7 +153,7 @@ func TestScanRepositoryCmd_Run(t *testing.T) {
expectedVersionUpdatesInBranch: map[string][]string{"frogbot-update-68d9dee2475e5986e783d85dfa11baa0-dependencies-master": {"1.2.6", "0.8.4"}},
packageDescriptorPaths: []string{"package.json", "inner-project/package.json"},
aggregateFixes: true,
allowPartialResults: true,
failUponAnyScannerError: false,
},
}
baseDir, err := os.Getwd()
Expand Down Expand Up @@ -202,20 +190,20 @@ func TestScanRepositoryCmd_Run(t *testing.T) {
repository, err := utils.BuildRepositoryFromEnv(xrayVersion, xscVersion, client, &gitTestParams, &serverParams, utils.ScanRepository)
assert.NoError(t, err)

// We must set a non-nil config profile to avoid panic
repository.ConfigProfile = &emptyConfigProfile
configProfile := createTestConfigProfile(test.aggregateFixes, test.failUponAnyScannerError)
repository.ConfigProfile = &configProfile

// Run
var cmd = ScanRepositoryCmd{XrayVersion: xrayVersion, XscVersion: xscVersion, dryRun: true, dryRunRepoPath: testDir}
defer func() { assert.NoError(t, os.Chdir(baseDir)) }()
err = cmd.Run(repository, client)
defer func() {
assert.NoError(t, os.Chdir(baseDir))
}()
// Restore CWD immediately so Windows releases the directory handle before the next subtest runs.
assert.NoError(t, os.Chdir(baseDir))

// Validate
repoDir := filepath.Join(testDir, test.testName)
assert.NoError(t, err)
for branch, packages := range test.expectedPackagesInBranch {
resultDiff, err := verifyDependencyFileDiff("master", branch, test.packageDescriptorPaths...)
resultDiff, err := verifyDependencyFileDiff(repoDir, "master", branch, test.packageDescriptorPaths...)
assert.NoError(t, err)
if len(packages) > 0 {
assert.NotEmpty(t, resultDiff)
Expand All @@ -231,7 +219,7 @@ func TestScanRepositoryCmd_Run(t *testing.T) {

if len(test.expectedMissingFilesInBranch) > 0 {
for branch, expectedMissingFiles := range test.expectedMissingFilesInBranch {
resultDiff, err := verifyLockFileDiff(branch, expectedMissingFiles...)
resultDiff, err := verifyLockFileDiff(repoDir, branch, expectedMissingFiles...)
assert.NoError(t, err)
assert.Empty(t, resultDiff)
}
Expand Down Expand Up @@ -331,15 +319,14 @@ pr body
gitTestParams.Branches = []string{"master"}
repository, err := utils.BuildRepositoryFromEnv(xrayVersion, xscVersion, client, gitTestParams, &serverParams, utils.ScanRepository)
assert.NoError(t, err)
// We must set a non-nil config profile to avoid panic
repository.ConfigProfile = &emptyConfigProfile
configProfile := createTestConfigProfile(true, false)
repository.ConfigProfile = &configProfile

// Run
var cmd = ScanRepositoryCmd{dryRun: true, dryRunRepoPath: testDir}
defer func() { assert.NoError(t, os.Chdir(baseDir)) }()
err = cmd.Run(repository, client)
defer func() {
assert.NoError(t, os.Chdir(baseDir))
}()
// Restore CWD immediately so Windows releases the directory handle before the next subtest runs.
assert.NoError(t, os.Chdir(baseDir))
assert.NoError(t, err)
})
}
Expand Down Expand Up @@ -426,11 +413,12 @@ func TestPackageTypeFromScan(t *testing.T) {
for _, file := range files {
log.Info(file)
}
configProfile := createTestConfigProfile(false, false)
scanSetup := utils.ScanDetails{
XrayVersion: xrayVersion,
XscVersion: xscVersion,
ServerDetails: &frogbotParams.Server,
ConfigProfile: &emptyConfigProfile,
ConfigProfile: &configProfile,
}
testScan.scanDetails = &scanSetup
scanResponse, err := testScan.scan()
Expand Down Expand Up @@ -779,41 +767,41 @@ func verifyTechnologyNaming(t *testing.T, scanResponse []services.ScanResponse,
}

// Executing git diff to ensure that the intended changes to the dependent file have been made
func verifyDependencyFileDiff(baseBranch string, fixBranch string, packageDescriptorPaths ...string) (output []byte, err error) {
func verifyDependencyFileDiff(repoDir string, baseBranch string, fixBranch string, packageDescriptorPaths ...string) (output []byte, err error) {
log.Debug(fmt.Sprintf("Checking differences in %s between branches %s and %s", packageDescriptorPaths, baseBranch, fixBranch))
// Suppress condition always false warning
//goland:noinspection ALL
var args []string
var cmd *exec.Cmd
if coreutils.IsWindows() {
args = []string{"/c", "git", "diff", baseBranch, fixBranch}
args = append(args, packageDescriptorPaths...)
output, err = exec.Command("cmd", args...).Output()
args := append([]string{"/c", "git", "diff", baseBranch, fixBranch}, packageDescriptorPaths...)
cmd = exec.Command("cmd", args...)
} else {
args = []string{"diff", baseBranch, fixBranch}
args = append(args, packageDescriptorPaths...)
output, err = exec.Command("git", args...).Output()
args := append([]string{"diff", baseBranch, fixBranch}, packageDescriptorPaths...)
cmd = exec.Command("git", args...)
}
cmd.Dir = repoDir
output, err = cmd.Output()
var exitError *exec.ExitError
if errors.As(err, &exitError) {
err = errors.New("git error: " + string(exitError.Stderr))
}
return
}

func verifyLockFileDiff(branchToInspect string, lockFiles ...string) (output []byte, err error) {
func verifyLockFileDiff(repoDir string, branchToInspect string, lockFiles ...string) (output []byte, err error) {
log.Debug(fmt.Sprintf("Checking lock files differences in %s between branches 'master' and '%s'", lockFiles, branchToInspect))
// Suppress condition always false warning
//goland:noinspection ALL
var args []string
var cmd *exec.Cmd
if coreutils.IsWindows() {
args = []string{"/c", "git", "ls-tree", branchToInspect, "--"}
args = append(args, lockFiles...)
output, err = exec.Command("cmd", args...).Output()
args := append([]string{"/c", "git", "ls-tree", branchToInspect, "--"}, lockFiles...)
cmd = exec.Command("cmd", args...)
} else {
args = []string{"ls-tree", branchToInspect, "--"}
args = append(args, lockFiles...)
output, err = exec.Command("git", args...).Output()
args := append([]string{"ls-tree", branchToInspect, "--"}, lockFiles...)
cmd = exec.Command("git", args...)
}
cmd.Dir = repoDir
output, err = cmd.Output()
var exitError *exec.ExitError
if errors.As(err, &exitError) {
err = errors.New("git error: " + string(exitError.Stderr))
Expand Down Expand Up @@ -849,7 +837,8 @@ func createScanRepoGitHubHandler(t *testing.T, port *string, response interface{
if r.RequestURI == fmt.Sprintf("/%s", projectName) {
file, err := os.ReadFile(fmt.Sprintf("%s.tar.gz", projectName))
assert.NoError(t, err)
_, err = w.Write(file)
w.Header().Set("Content-Type", "application/octet-stream")
_, err = io.Copy(w, bytes.NewReader(file))
assert.NoError(t, err)
return
}
Expand Down Expand Up @@ -892,3 +881,26 @@ func createScanRepoGitHubHandler(t *testing.T, port *string, response interface{
}
}
}

func createTestConfigProfile(aggregateFixes, failUponAnyScannerError bool) services2.ConfigProfile {
return services2.ConfigProfile{
ProfileName: "test-profile",
FrogbotConfig: services2.FrogbotConfig{
CreateAutoFixPr: true,
AggregateFixes: aggregateFixes,
},
GeneralConfig: services2.GeneralConfig{
FailUponAnyScannerError: failUponAnyScannerError,
},
Modules: []services2.Module{{
ModuleId: 0,
ModuleName: "test-module",
PathFromRoot: ".",
ScanConfig: services2.ScanConfig{
ScaScannerConfig: services2.ScaScannerConfig{
EnableScaScan: true,
},
},
}},
}
}
37 changes: 24 additions & 13 deletions utils/testsutils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ package utils
import (
"encoding/json"
"fmt"
"io"
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"runtime"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -74,7 +74,20 @@ func CopyTestdataProjectsToTemp(t *testing.T, testDir string) (tmpDir string, re
err = biutils.CopyDir(filepath.Join("..", "testdata", testDir), tmpDir, true, []string{})
assert.NoError(t, err)
restoreFunc = func() {
assert.NoError(t, fileutils.RemoveTempDir(tmpDir))
err := fileutils.RemoveTempDir(tmpDir)
if err == nil {
return
}
// On Windows, directory cleanup can fail due to OS file-system locks held by
// background services (e.g., Windows Defender Real-time Protection, Search Indexer).
// This is a known issue: https://github.com/golang/go/issues/30789
// jfrog-client-go's RemoveTempDir already falls back to RemoveDirContents for this case,
// and CleanOldDirs() will remove the leftover empty directory on the next run.
if runtime.GOOS == "windows" {
t.Logf("Warning: temp dir cleanup failed on Windows (will be retried by CleanOldDirs): %v", err)
return
}
assert.NoError(t, err)
}
return
}
Expand Down Expand Up @@ -184,18 +197,22 @@ func CreateXscMockServerForConfigProfile(t *testing.T, xrayVersion string) (mock
updatedContent = strings.Replace(updatedContent, `"path_from_root": "."`, `"path_from_root": "backend"`, 1)
content = []byte(updatedContent)
}
var responseProfile services.ConfigProfile
assert.NoError(t, json.Unmarshal(content, &responseProfile))
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
_, err = w.Write(content)
assert.NoError(t, err)
assert.NoError(t, json.NewEncoder(w).Encode(responseProfile))

// Endpoint to profile by URL
case strings.Contains(r.RequestURI, "/xsc/profile_repos") && isXrayAfterXscMigration:
assert.Equal(t, http.MethodPost, r.Method)
content, err := os.ReadFile("../testdata/configprofile/configProfileExample.json")
assert.NoError(t, err)
var responseProfile services.ConfigProfile
assert.NoError(t, json.Unmarshal(content, &responseProfile))
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
_, err = w.Write(content)
assert.NoError(t, err)
assert.NoError(t, json.NewEncoder(w).Encode(responseProfile))

case r.RequestURI == fmt.Sprintf("/%s/%ssystem/version", apiUrlPart, "xsc"):
_, err := fmt.Fprintf(w, `{"xsc_version": "%s"}`, services.ConfigProfileMinXscVersion)
Expand Down Expand Up @@ -232,14 +249,8 @@ func CreateMockServerForDependencySubmission(t *testing.T, owner, repo string) *
}

// Read request body and parse it to ensure all mandatory fields exist
body, err := io.ReadAll(r.Body)
if err != nil {
t.Errorf("Failed to read request body: %v", err)
w.WriteHeader(http.StatusBadRequest)
return
}
var snapshot map[string]interface{}
if err := json.Unmarshal(body, &snapshot); err != nil {
if err := json.NewDecoder(r.Body).Decode(&snapshot); err != nil {
t.Errorf("Failed to parse request body as JSON: %v", err)
w.WriteHeader(http.StatusBadRequest)
return
Expand Down
Loading