test: use custom filter to make tests less brittle

[acpana]

Re working some of these to be less brittle. It's going to help any time we make changes to the Result type in CF.

Signed-off-by: Alex Pana 8968914+acpana@users.noreply.github.com

[codecov-commenter]

Codecov Report

Base: 53.62% // Head: 53.51% // Decreases project coverage by -0.12% ⚠️

Coverage data is based on head (20f93a7) compared to base (8170c5f).
Patch has no changes to coverable lines.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2544      +/-   ##
==========================================
- Coverage   53.62%   53.51%   -0.12%     
==========================================
  Files         120      120              
  Lines       10634    10634              
==========================================
- Hits         5703     5691      -12     
- Misses       4500     4509       +9     
- Partials      431      434       +3     

Flag	Coverage Δ
unittests	53.51% <ø> (-0.12%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...onstrainttemplate/constrainttemplate_controller.go	54.06% <0.00%> (-2.88%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[JaydipGabani]

@acpana can you explain why this was brittle? What was causing it to break? Just curious

[acpana]

can you explain why this was brittle? What was causing it to break?

the tl;dr issue imo is that we are comparing structs absolutely (i.e. all the fields). you'll see that we have a few cmpopts.IgnoreFields() calls in our testing to ignore struct fields that we don't necessarily care about or can compare absolutely.

When working on the structs themselves, like Result, the current approach causes test failures. You won't see those failures here, but you will see them in the constraint framework repo. eg job runs:

• https://github.com/open-policy-agent/frameworks/actions/runs/4022670603/jobs/6912655374

does this help offer more context?

[JaydipGabani]

can you explain why this was brittle? What was causing it to break?

the tl;dr issue imo is that we are comparing structs absolutely (i.e. all the fields). you'll see that we have a few cmpopts.IgnoreFields() calls in our testing to ignore struct fields that we don't necessarily care about or can compare absolutely.

When working on the structs themselves, like Result, the current approach causes test failures. You won't see those failures here, but you will see them in the constraint framework repo. eg job runs:

• https://github.com/open-policy-agent/frameworks/actions/runs/4022670603/jobs/6912655374

does this help offer more context?

Thanks for clearing it up

[acpana]

the main difference here between our custom option and the IgnoreFields option is that the latter relies the existence of a field; ref: https://github.com/google/go-cmp/blob/a97318bf6562f2ed2632c5f985db51b1bc5bdcd0/cmp/cmpopts/struct_filter.go#L177-L179

[acpana]

@maxsmythe @sozercan @ritazh PTAL 🙏🏼

[maxsmythe]

LGTM

[sozercan]

Thanks! LGTM