Fix unreleased bugs with labels_include_any feature
#23734
Conversation
gillespi314
temporarily deployed
to
Docker Hub
GitHub Actions
Inactive
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feat-include-any-label #23734 +/- ##
=========================================================
Coverage ? 63.10%
=========================================================
Files ? 1556
Lines ? 147306
Branches ? 3724
=========================================================
Hits ? 92963
Misses ? 46979
Partials ? 7364
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
gillespi314
had a problem deploying
to
Docker Hub
GitHub Actions
Error
| // FIXME: At what point are we deleting all labels for a profile (e.g., the user might | ||
| // remove all labels from an existing profile)? |
There was a problem hiding this comment.
This is another released bug that I found while testing. In the interest of keeping this feature branch moving, I'll file a separate bug ticket for this issue.
| // FIXME: equivalent of no label condition, should clear all labels slice? | ||
| // p.Labels = nil | ||
| // p.LabelsIncludeAll = nil | ||
| // p.LabelsIncludeAny = nil | ||
| // p.LabelsExcludeAny = nil |
There was a problem hiding this comment.
This is another released bug found while testing. In the interest of keeping this feature branch moving, I'll file a separate bug ticket for this issue.
| // FIXME: At what point are we deleting label associations for existing profiles (e.g. if the user | ||
| // removes all labels from a profile in gitops, shouldn't we remove the old associations)? |
There was a problem hiding this comment.
This is another released bug found while testing. In the interest of keeping this feature branch moving, I'll file a separate bug ticket for this issue.
| @@ -333,6 +368,67 @@ func TestMDMProfileSpecUnmarshalJSON(t *testing.T) { | |||
| require.Equal(t, "oldpath", p.Path) | |||
| require.Empty(t, p.Labels) | |||
| }) | |||
|
|
|||
| t.Run("changing labels", func(t *testing.T) { | |||
There was a problem hiding this comment.
This was a released bug found during testing. I opted to fix this one, but there are a few others that I'm planning to file as separate issues to keep this feature branch moving forward.
gillespi314
temporarily deployed
to
Docker Hub
GitHub Actions
Inactive
gillespi314
temporarily deployed
to
Docker Hub
GitHub Actions
Inactive
|
Failing test is unrelated and known to be flaky |
Issue #23539
Checklist for submitter
If some of the following don't apply, delete the relevant line.