chore(test): assert info state mutations and build errors in TestFlux- Fixes #3409#3410
chore(test): assert info state mutations and build errors in TestFlux- Fixes #3409#3410gojogourav wants to merge 2 commits intokubeflow:masterfrom
Conversation
Why: - Previously, the TestFlux table-driven tests only verified the final generated Kubernetes objects and container properties. - Expanded the test cases to explicitly assert against wantInfo, wantMLPolicyError, and wantBuildError. - This ensures that EnforceMLPolicy and Build correctly mutate the internal runtime.Info state and bubble up appropriate error values, preventing silent regressions in the plugin's state management. Signed-off-by: gojogourav <gojogourav@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
This PR expands the TestFlux table-driven tests to explicitly assert against Info state mutations and error values returned from the plugin's EnforceMLPolicy and Build methods. Previously, the tests only verified the final generated Kubernetes objects and container properties. The changes replace fatal error assertions with comparative error checks and add detailed state verification.
Changes:
- Add
wantInfo,wantMLPolicyError, andwantBuildErrorfields to the test case struct - Replace
t.Fatalferror assertions withgocmp.Diffcomparisons usingcmpopts.EquateErrors() - Add comprehensive
Infostate comparison afterEnforceMLPolicycalls with appropriate comparison options
| "flux mutations are applied correctly": { | ||
|
|
||
| wantInfo: &runtime.Info{ | ||
| RuntimePolicy: runtime.RuntimePolicy{ | ||
| MLPolicySource: &trainer.MLPolicySource{ | ||
| Flux: &trainer.FluxMLPolicySource{ | ||
| NumProcPerNode: &procs, | ||
| }, | ||
| }, | ||
| }, | ||
| TemplateSpec: runtime.TemplateSpec{ | ||
| PodSets: []runtime.PodSet{ | ||
| { | ||
| Name: constants.Node, | ||
| Ancestor: ptr.To(constants.AncestorTrainer), | ||
| Count: ptr.To[int32](1), | ||
| }, | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
The second test case is missing explicit wantMLPolicyError and wantBuildError field assignments for consistency with the first test case. Add wantMLPolicyError: nil and wantBuildError: nil to clarify the expected behavior.
Signed-off-by: gojogourav <gojogourav@gmail.com>
gojogourav
changed the title
gojogourav
changed the title
gojogourav
changed the title
Why:
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in
Fixes #<issue number>, #<issue number>, ...format, will close the issue(s) when PR gets merged):Fixes #
Checklist: