Skip to content

ROX-29771: Refactor common nongroovy test code #17374

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
vikin91 merged 22 commits into from
Nov 19, 2025
Merged

ROX-29771: Refactor common nongroovy test code #17374

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
ef82409
X-Smart-Branch-Parent: master
vikin91 Nov 19, 2025
ecd9f51
Improve test robustness: refactor deployment setup and fix TestExclud…
vikin91 Oct 17, 2025
cf4e2ac
Fix TestExcludedScopes compilation: use waitForDeploymentInCentral
vikin91 Oct 17, 2025
dd242dc
Unify usage of testing.T and testutils.T
vikin91 Oct 17, 2025
0dcb982
Update missed refs to T
vikin91 Oct 17, 2025
6d0ceb0
Fix roxvet issues
vikin91 Oct 17, 2025
593229e
Revert image URLs to docker.io/nginx for independence from prefetch i…
vikin91 Oct 20, 2025
c1731af
Readd approach with testutilsLogger + document why
vikin91 Oct 23, 2025
070ca5e
Fix post-rebase compliation issues
vikin91 Oct 24, 2025
9c21b06
Readd changing imagePullPolicy to never for prefetched images
vikin91 Oct 29, 2025
b310b20
Use doch prefetched images
vikin91 Oct 29, 2025
f6bc7de
Self-review part 1
vikin91 Oct 29, 2025
1a08eb4
Ensure that all nongroovy tests use t.Logf for logging
vikin91 Oct 29, 2025
cc022a1
Relax conditions for teardownDeployment
vikin91 Oct 29, 2025
01a51aa
Ensure that teardownDeployment is retried
vikin91 Oct 29, 2025
c0deb8e
Ensure waitForDeploymentReadyInK8s retries on errors
vikin91 Oct 30, 2025
ed8e257
Add namespace as arg to teardownDeployment
vikin91 Oct 30, 2025
6842fba
Make createDeploymentViaAPI return an error
vikin91 Oct 30, 2025
8c0e45d
Add tests/common.go to large files :(
vikin91 Oct 30, 2025
212bdf2
Add nil-check
vikin91 Oct 30, 2025
a685ee9
Tune a comment
vikin91 Nov 11, 2025
d4d8c77
Make error in deleteFunc retriable
vikin91 Nov 19, 2025
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
6 changes: 3 additions & 3 deletions tests/backup_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ func TestBackup(t *testing.T) {
}
deploymentName := fmt.Sprintf("test-backup-%d", rand.Intn(10000))

setupDeployment(t, "quay.io/rhacs-eng/qa-multi-arch:nginx-1.21.1", deploymentName)
defer teardownDeploymentWithoutCheck(t, deploymentName)
waitForDeployment(t, deploymentName)
setupDeploymentInNamespace(t, "quay.io/rhacs-eng/qa-multi-arch:nginx-1.21.1", deploymentName, "default")
defer teardownDeploymentWithoutCheck(t, deploymentName, "default")
waitForDeploymentInCentral(t, deploymentName)

for _, includeCerts := range []bool{false, true} {
t.Run(fmt.Sprintf("includeCerts=%t", includeCerts), func(t *testing.T) {
Expand Down
8 changes: 4 additions & 4 deletions tests/cluster_delete_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func TestClusterDeletion(t *testing.T) {
assert.NotZero(t, counts.DeploymentCount)
assert.NotZero(t, counts.SecretCount)
assert.NotZero(t, counts.PodCount)
log.Infof("the initial counts are: %+v", counts)
t.Logf("the initial counts are: %+v", counts)

conn := centralgrpc.GRPCConnectionToCentral(t)
service := v1.NewClustersServiceClient(conn)
Expand All @@ -100,12 +100,12 @@ func TestClusterDeletion(t *testing.T) {
counts := getAllCounts(t)
if counts.DeploymentCount == 0 {
if counts.AllZero() {
log.Infof("objects have all drained to 0")
t.Logf("objects have all drained to 0")
return
}
log.Infof("resp still has non zero values: %+v", counts)
t.Logf("resp still has non zero values: %+v", counts)
} else {
log.Infof("deployment count is still not zero: %d", counts.DeploymentCount)
t.Logf("deployment count is still not zero: %d", counts.DeploymentCount)
}

if previous.DeploymentCount > 0 && previous.DeploymentCount > counts.DeploymentCount {
Expand Down
323 changes: 249 additions & 74 deletions tests/common.go
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion tests/compliance_operator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func getCurrentComplianceResults(t testutils.T) (rhcos, ocp *storage.ComplianceR
for _, run := range statusRunResp.GetRuns() {
if run.GetState() != v1.ComplianceRun_FINISHED {
finished = false
log.Infof("Run for %v is in state %v", run.GetStandardId(), run.GetState())
t.Logf("Run for %v is in state %v", run.GetStandardId(), run.GetState())
}
}
if finished {
Expand Down
14 changes: 7 additions & 7 deletions tests/compliance_operator_v2_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func scaleToN(ctx context.Context, client kubernetes.Interface, deploymentName s
return nil
}

func createDynamicClient(t T) dynclient.Client {
func createDynamicClient(t testutils.T) dynclient.Client {
restCfg := getConfig(t)
restCfg.WarningHandler = rest.NoWarnings{}
k8sClient := createK8sClient(t)
Expand Down Expand Up @@ -138,7 +138,7 @@ func waitForComplianceSuiteToComplete(t *testing.T, suiteName string, interval,
ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel()

log.Info("Waiting for ComplianceSuite to reach DONE phase")
t.Logf("Waiting for ComplianceSuite to reach DONE phase")
for range ticker.C {
var suite complianceoperatorv1.ComplianceSuite
mustEventually(t, ctx, func() error {
Expand All @@ -147,10 +147,10 @@ func waitForComplianceSuiteToComplete(t *testing.T, suiteName string, interval,
}, timeout, fmt.Sprintf("failed to get ComplianceSuite %s", suiteName))

if suite.Status.Phase == "DONE" {
log.Infof("ComplianceSuite %s reached DONE phase", suiteName)
t.Logf("ComplianceSuite %s reached DONE phase", suiteName)
return
}
log.Infof("ComplianceSuite %s is in %s phase", suiteName, suite.Status.Phase)
t.Logf("ComplianceSuite %s is in %s phase", suiteName, suite.Status.Phase)
}
}

Expand All @@ -168,15 +168,15 @@ func cleanUpResources(ctx context.Context, t *testing.T, resourceName string, na
}
}

func assertResourceDoesExist(ctx context.Context, t T, resourceName string, namespace string, obj dynclient.Object) dynclient.Object {
func assertResourceDoesExist(ctx context.Context, t testutils.T, resourceName string, namespace string, obj dynclient.Object) dynclient.Object {
client := createDynamicClient(t)
require.Eventually(t, func() bool {
return client.Get(ctx, types.NamespacedName{Name: resourceName, Namespace: namespace}, obj) == nil
}, 60*time.Second, 10*time.Millisecond)
return obj
}

func assertResourceWasUpdated(ctx context.Context, t T, resourceName string, namespace string, obj dynclient.Object) dynclient.Object {
func assertResourceWasUpdated(ctx context.Context, t testutils.T, resourceName string, namespace string, obj dynclient.Object) dynclient.Object {
client := createDynamicClient(t)
oldResourceVersion := obj.GetResourceVersion()
require.Eventually(t, func() bool {
Expand All @@ -185,7 +185,7 @@ func assertResourceWasUpdated(ctx context.Context, t T, resourceName string, nam
return obj
}

func assertResourceDoesNotExist(ctx context.Context, t T, resourceName string, namespace string, obj dynclient.Object) {
func assertResourceDoesNotExist(ctx context.Context, t testutils.T, resourceName string, namespace string, obj dynclient.Object) {
client := createDynamicClient(t)
require.Eventually(t, func() bool {
err := client.Get(ctx, types.NamespacedName{Name: resourceName, Namespace: namespace}, obj)
Expand Down
2 changes: 1 addition & 1 deletion tests/container_instances_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func getGroupedContainerInstances(t testutils.T, podID string) []ContainerNameGr
`, map[string]interface{}{
"containersQuery": fmt.Sprintf("Pod ID: %s", podID),
}, &respData, timeout)
log.Info(respData)
t.Logf("%+v", respData)

return respData.GroupedContainerInstances
}
11 changes: 7 additions & 4 deletions tests/delegated_scanning_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,7 @@ func (ts *DelegatedScanningSuite) TestDeploymentScans() {
t := ts.T()

// Do an initial teardown in case a deployment is lingering from a previous test.
teardownDeployment(t, deployName)
teardownDeployment(t, deployName, "default")

// Since we cannot 'force' a scan when deploying an image, we first delete
// the image to help ensure a fresh scan is executed. Otherwise Sensor
Expand All @@ -686,7 +686,7 @@ func (ts *DelegatedScanningSuite) TestDeploymentScans() {

// Only perform teardown on success so that logs can be captured on failure.
logf(t, "Tearing down deployment %q", deployName)
teardownDeploymentWithoutCheck(t, deployName)
teardownDeploymentWithoutCheck(t, deployName, "default")
}

ts.Run("scan deployed image", func() {
Expand Down Expand Up @@ -838,7 +838,7 @@ func (ts *DelegatedScanningSuite) TestMirrorScans() {
}

// Do an initial teardown in case a deployment is lingering from a previous test.
teardownDeployment(t, tc.deployName)
teardownDeployment(t, tc.deployName, "default")

// Because we cannot 'force' a scan for deployments, we explicitly delete the image
// so that it is removed from Sensor scan cache.
Expand All @@ -862,7 +862,10 @@ func (ts *DelegatedScanningSuite) TestMirrorScans() {

// Only perform teardown on success so that logs can be captured on failure.
logf(t, "Tearing down deployment %q", tc.deployName)
teardownDeployment(t, tc.deployName)
// Wait for deployment to be fully ready before deletion to avoid race conditions
// where deletion is issued while pods are still starting up.
waitForDeploymentReadyInK8s(t, tc.deployName, "default")
teardownDeployment(t, tc.deployName, "default")
})
}
}
Expand Down
8 changes: 4 additions & 4 deletions tests/excluded_scopes_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ func TestExcludedScopes(t *testing.T) {
}
deploymentName := fmt.Sprintf("test-excluded-scopes-%d", rand.Intn(10000))

setupDeployment(t, "quay.io/rhacs-eng/qa-multi-arch-nginx:latest", deploymentName)
defer teardownDeploymentWithoutCheck(t, deploymentName)
waitForDeployment(t, deploymentName)
setupDeploymentInNamespace(t, "quay.io/rhacs-eng/qa-multi-arch-nginx:latest", deploymentName, "default")
defer teardownDeploymentWithoutCheck(t, deploymentName, "default")
waitForDeploymentInCentral(t, deploymentName)

verifyNoAlertForExcludedScopes(t, deploymentName)
verifyAlertForExcludedScopesRemoval(t, deploymentName)
Expand All @@ -50,7 +50,7 @@ func waitForAlert(t *testing.T, service v1.AlertServiceClient, req *v1.ListAlert
for _, alert := range alerts {
alertStrings = fmt.Sprintf("%s%s\n", alertStrings, protocompat.MarshalTextString(alert))
}
log.Infof("Received alerts:\n%s", alertStrings)
t.Logf("Received alerts:\n%s", alertStrings)
require.Fail(t, fmt.Sprintf("Failed to have %d alerts, instead received %d alerts", desired, len(alerts)))
}

Expand Down
4 changes: 2 additions & 2 deletions tests/external_backup_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func TestGCSExternalBackup(t *testing.T) {
time.Sleep(10 * time.Second)
}),
retry.OnFailedAttempts(func(err error) {
log.Error(err.Error())
t.Logf("Error testing external backup: %v", err)
}),
)
assert.NoError(t, err)
Expand Down Expand Up @@ -137,7 +137,7 @@ func TestGCSExternalBackup(t *testing.T) {
time.Sleep(1 * time.Second)
}),
retry.OnFailedAttempts(func(err error) {
log.Error(err.Error())
t.Logf("Error waiting for backup pruning: %v", err)
}),
)
require.NoError(t, err)
Expand Down
16 changes: 8 additions & 8 deletions tests/pods_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,13 @@ func setupMultiContainerPodTest(t *testing.T) (*coreV1.Pod, string, Pod, func())
ensurePodExists(retryT, client, kPod)
// Wait for pod to be fully running with all containers ready
k8sPod = waitForPodRunning(retryT, client, kPod.GetNamespace(), kPod.GetName())
t.Logf("Pod %s is running with all containers ready", kPod.GetName())
retryT.Logf("Pod %s is running with all containers ready", kPod.GetName())

// Now wait for Central to see the deployment
// This can take time as Sensor needs to detect the pod and report it to Central
t.Logf("Waiting for Central to see deployment %s", kPod.GetName())
waitForDeployment(retryT, kPod.GetName())
t.Logf("Central now sees deployment %s", kPod.GetName())
retryT.Logf("Waiting for Central to see deployment %s", kPod.GetName())
waitForDeploymentInCentral(retryT, kPod.GetName())
retryT.Logf("Central now sees deployment %s", kPod.GetName())
})

deploymentID := ""
Expand Down Expand Up @@ -183,7 +183,7 @@ func getDeploymentID(t testutils.T, deploymentName string) string {
`, map[string]interface{}{
"query": fmt.Sprintf("Deployment: %s", deploymentName),
}, &respData, timeout)
log.Info(respData)
t.Logf("%+v", respData)
require.Len(t, respData.Deployments, 1)

return string(respData.Deployments[0].ID)
Expand Down Expand Up @@ -223,7 +223,7 @@ func getPods(t testutils.T, deploymentID string) []Pod {
"podsQuery": fmt.Sprintf("Deployment ID: %s", deploymentID),
"pagination": pagination,
}, &respData, timeout)
log.Infof("%+v", respData)
t.Logf("%+v", respData)

return respData.Pods
}
Expand All @@ -242,7 +242,7 @@ func getPodCountInCentral(t testutils.T, deploymentID string) int {
`, map[string]interface{}{
"podsQuery": fmt.Sprintf("Deployment ID: %s", deploymentID),
}, &respData, timeout)
log.Infof("Pod count in Central for deployment %s: %d", deploymentID, respData.PodCount)
t.Logf("Pod count in Central for deployment %s: %d", deploymentID, respData.PodCount)

return int(respData.PodCount)
}
Expand All @@ -269,7 +269,7 @@ func getEvents(t testutils.T, pod Pod) []Event {
`, map[string]interface{}{
"podId": pod.ID,
}, &respData, timeout)
log.Infof("Get Events: %+v", respData)
t.Logf("Get Events: %+v", respData)

return respData.Pod.Events
}
12 changes: 6 additions & 6 deletions tests/policy_as_code_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ func (pc *PolicyAsCodeSuite) TestRenameToDefaultCR() {

func (pc *PolicyAsCodeSuite) createPolicyInCentral() *storage.Policy {
policyName := "This is a test policy"
log.Infof("Adding policy with name \"%s\"", policyName)
pc.T().Logf("Adding policy with name \"%s\"", policyName)
policy, err := pc.policyClient.PostPolicy(pc.ctx, &v1.PostPolicyRequest{
Policy: &storage.Policy{
Name: policyName,
Expand All @@ -283,15 +283,15 @@ func (pc *PolicyAsCodeSuite) createPolicyInCentral() *storage.Policy {
}

func (pc *PolicyAsCodeSuite) createClusterInCentral() *storage.Cluster {
log.Infof("Adding cluster with name \"%s\"", clusterName)
pc.T().Logf("Adding cluster with name \"%s\"", clusterName)
cluster, err := pc.clusterClient.PostCluster(pc.ctx, &storage.Cluster{Name: fixtureconsts.ClusterName1, MainImage: "docker.io/stackrox/rox:latest", CentralApiEndpoint: "central.stackrox:443"})
pc.NoError(err)
return cluster.GetCluster()
}

func (pc *PolicyAsCodeSuite) createNotifierInCentral() *storage.Notifier {
notifierName := "email-notifier"
log.Infof("Adding notifier with name \"%s\"", notifierName)
pc.T().Logf("Adding notifier with name \"%s\"", notifierName)
notifier, err := pc.notifierClient.PostNotifier(pc.ctx, &storage.Notifier{
Name: notifierName,
Type: notifiers.EmailType,
Expand Down Expand Up @@ -474,15 +474,15 @@ func (pc *PolicyAsCodeSuite) createCRAndObserveInCentral(policyCR *v1alpha1.Secu
func (pc *PolicyAsCodeSuite) TearDownSuite() {
// TODO: Don't double delete
for _, policy := range pc.policies {
log.Infof("Deleting policy with name \"%s\"", policy.GetName())
pc.T().Logf("Deleting policy with name \"%s\"", policy.GetName())
_, err := pc.policyClient.DeletePolicy(pc.ctx, &v1.ResourceByID{
Id: policy.GetId(),
})
pc.Require().NoError(err)
}

if pc.notifier != nil {
log.Infof("Deleting notifier with name \"%s\"", pc.notifier.GetName())
pc.T().Logf("Deleting notifier with name \"%s\"", pc.notifier.GetName())
_, err := pc.notifierClient.DeleteNotifier(pc.ctx, &v1.DeleteNotifierRequest{
Id: pc.notifier.GetId(),
Force: true,
Expand All @@ -491,7 +491,7 @@ func (pc *PolicyAsCodeSuite) TearDownSuite() {
}

if pc.cluster != nil {
log.Infof("Deleting cluster with name \"%s\"", pc.cluster.GetName())
pc.T().Logf("Deleting cluster with name \"%s\"", pc.cluster.GetName())
_, err := pc.clusterClient.DeleteCluster(pc.ctx, &v1.ResourceByID{
Id: pc.cluster.GetId(),
})
Expand Down
6 changes: 3 additions & 3 deletions tests/policy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func tearDownImportExportTest(t *testing.T) {

var cleanupErrors []error
for _, id := range addedPolicies {
log.Infof("Added policy: %s", id)
t.Logf("Added policy: %s", id)
if id == knownPolicyID {
continue
}
Expand All @@ -77,7 +77,7 @@ func tearDownImportExportTest(t *testing.T) {
})
cancel()
if err != nil {
log.Infof("error deleting policy %s, error: %v", id, err)
t.Logf("error deleting policy %s, error: %v", id, err)
cleanupErrors = append(cleanupErrors, err)
}
}
Expand Down Expand Up @@ -135,7 +135,7 @@ func validateImport(t *testing.T, importResp *v1.ImportPoliciesResponse, policie

func validateSuccess(t *testing.T, importPolicyResponse *v1.ImportPolicyResponse, expectedPolicy *storage.Policy, ignoreID bool) {
require.True(t, importPolicyResponse.GetSucceeded())
log.Infof("Adding policy %s with id: %s", importPolicyResponse.GetPolicy().GetName(), importPolicyResponse.GetPolicy().GetId())
t.Logf("Adding policy %s with id: %s", importPolicyResponse.GetPolicy().GetName(), importPolicyResponse.GetPolicy().GetId())
addedPolicies = append(addedPolicies, importPolicyResponse.GetPolicy().GetId())
if ignoreID {
expectedPolicy.Id = ""
Expand Down
6 changes: 3 additions & 3 deletions tests/resourcecollection_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,23 +137,23 @@ func (s *CollectionE2ETestSuite) SetupSuite() {

// wait for deployments to propagate
qb := search.NewQueryBuilder().AddRegexes(search.Namespace, "collections-test-.")
waitForDeploymentCount(s.T(), qb.Query(), len(collectionNamespaces)*len(testDeployments))
waitForDeploymentCountInCentral(s.T(), qb.Query(), len(collectionNamespaces)*len(testDeployments))
}

func (s *CollectionE2ETestSuite) TearDownSuite() {
// clean up namespaces
for _, ns := range collectionNamespaces {
err := s.nsClient.Delete(s.ctx, ns, metav1.DeleteOptions{})
if err != nil {
log.Errorf("failed deleting %q testing namespace %q", ns, err)
s.T().Logf("failed deleting %q testing namespace %q", ns, err)
}
}

// clean up collections
for _, id := range s.collectionIDs {
_, err := s.service.DeleteCollection(s.ctx, &v1.ResourceByID{Id: id})
if err != nil {
log.Errorf("failed deleting %q testing collection %q", id, err)
s.T().Logf("failed deleting %q testing collection %q", id, err)
}
}
}
Expand Down
1 change: 1 addition & 0 deletions tools/allowed-large-files
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ scanner/enricher/nvd/testdata/feed.json
scripts/ci/lib.sh
sensor/tests/data/policies.json
sensor/upgrader/preflight/testdata/k8s-1.22.json.gz
tests/common.go
tests/e2e/lib.sh
tests/e2e/run-scanner-v4-install.bats
tests/performance/load/package-lock.json
Expand Down
Loading