Add exclusion rules to filter unwanted events #259
Conversation
|
Thanks for the contribution! Before we can merge this, we need @acbox to sign the Salesforce Inc. Contributor License Agreement. |
|
could possibly solve #206 ? |
|
can the filtering also exclude CRD's ? |
Tiltfile
Outdated
| @@ -0,0 +1,57 @@ | |||
| # | |||
There was a problem hiding this comment.
Can you please move this file out to a separate PR, we'd like to keep PRs focused on one feature, thanks!
It would also be great if we can update readme with instructions on how to run tile with sloop.
There was a problem hiding this comment.
Removed, not sure this warrants a separate PR yet as there were some unresolved issues with the workflow incorporating Tilt. The Tiltfile also relied on running a local cluster with Kind, which is perhaps too opinionated.
pkg/sloop/common/utilities.go
Outdated
| @@ -37,3 +37,13 @@ func Contains(stringList []string, elem string) bool { | |||
| func GetFilePath(filePath string, fileName string) string { | |||
| return path.Join(filePath, fileName) | |||
| } | |||
|
|
|||
| func Truncate(text string, width int) (string, error) { | |||
There was a problem hiding this comment.
can you please add unit tests for Truncate?
There was a problem hiding this comment.
Added unit tests, doing this highlighted some bugs in Truncate that I've fixed.
|
|
||
| if !strings.Contains(result1.Payload, `"name":"ns"`) { t.Error("Expected event not found") } | ||
| if !strings.Contains(result2.Payload, `"name":"s1"`) { t.Error("Expected event not found") } | ||
| if !strings.Contains(result3.Payload, `"name":"s3"`) { t.Error("Expected event not found") } |
There was a problem hiding this comment.
should we check that "name":"s2" is not present?
There was a problem hiding this comment.
Could you give an example? I found this the most straightforward way. I changed the if statements to assert calls, bit cleaner.
| @@ -81,21 +82,58 @@ func Test_bigPicture(t *testing.T) { | |||
| masterURL := "url" | |||
| kubeContext := "" // empty string makes things work | |||
| enableGranularMetrics := true | |||
| kw, err := NewKubeWatcherSource(kubeClient, outChan, resync, includeCrds, time.Duration(10*time.Second), masterURL, kubeContext, enableGranularMetrics) | |||
| exclusionRules := map[string][]any{ | |||
There was a problem hiding this comment.
can we also add a test where exclusionRules are not provided, i.e. uses the default value of ExclusionRules: map[string][]any{},
There was a problem hiding this comment.
I've added separate tests for default and non-default exclusion rules
| return false | ||
| } | ||
| var result bytes.Buffer | ||
| err = jsonlogic.Apply( |
|
Pushed a fix for the failed GitHub Action test. |
Looks like the ordering for events received from the channel is non-deterministic. I had changed the tests in this PR to expect events in an exact order, which passed reliably on my local machine but failed when run by the GitHub Actions workflow. In this commit I've reverted the existing big picture test (the one with no exclusion rules) to not check the payload, and modified the new test (including an exclusion rule) to: a) wait 1 second for all events to be received on the channel b) verify that the excluded event is not received regardless of order c) verify that the expected number of events is received
|
This PR should pass the CI tests now as per the commit comment here ae0b4d0 |
|
@acbox thanks for your contribution. |
No description provided.