Skip to content

Comments

E2E "golden path" negative test case + test case: Try to update forbidden fields #4154

Draft
mvacula02 wants to merge 4 commits intomainfrom
mvacula/aro-23182
Draft

E2E "golden path" negative test case + test case: Try to update forbidden fields #4154
mvacula02 wants to merge 4 commits intomainfrom
mvacula/aro-23182

Conversation

@mvacula02
Copy link
Collaborator

ARO-23182

What

  1. Serves as a prototype for a "golden path" negative test case that encompasses all applicable simple negative test cases and runs them on a single cluster
  2. Implements test scenario: updating forbidden node pool platform profile fields
  3. Implements test scenario: updating node pool version to a version higher than its cluster BLOCKED: ARO-24542

This document discusses test cases that could be added to the "golden path".

Why

  • Most of the simple negative test cases do not need their own cluster, especially field validations
  • Incremental cost is incredibly low (usually 1 API call)

primarily targeting negative test case to update forbidden fields, but serves as a base for "golden path" negative test case, that runs various simple negative test cases on a single cluster
https://issues.redhat.com/browse/ARO-23182
@openshift-ci
Copy link

openshift-ci bot commented Feb 19, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mvacula02
Once this PR has been reviewed and has the lgtm label, please assign bennerv for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci
Copy link

openshift-ci bot commented Feb 19, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Copy link
Collaborator

@mbukatov mbukatov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving few comments.

modifiedNodePool.Properties.Platform.OSDisk.SizeGiB = to.Ptr[int32](256)
modifiedNodePool.Properties.Platform.OSDisk.DiskStorageAccountType = to.Ptr(hcpsdk20240610preview.DiskStorageAccountTypePremiumLRS)
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would simplify this a bit:

  • creating a new variable modifiedNodePool looks as if you copied the value, but this is not happening here, so you can just use the nodePool and modify it (unless you want to compare both, then you need to create new separate modifiedNodePool)
  • I would catch the cases where Properties == nil in the same level where you check err != nil in a branch above, to avoid nesting, and handled it as a separate case

}
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can also be simplified a bit (eg. we can ignore nil values, we don't branch to branch if we don't get error, ...). Also, we may want to make sure that the values we use to set a different value is actually different from the default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants