Add filterEventByAttributes tests

[pierDipi]

Proposed Changes

• Add filterEventByAttributes tests

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: pierDipi
To complete the pull request process, please assign n3wscott
You can assign the PR to them by writing /assign @n3wscott in a comment when ready.

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[knative-prow-robot]

@pierDipi: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-knative-eventing-unit-tests	da41123	link	/test pull-knative-eventing-unit-tests

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[knative-metrics-robot]

The following is the coverage report on the affected files.
Say /test pull-knative-eventing-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/mtbroker/filter/filter_handler.go	86.6%	88.1%	1.5

[pierDipi]

Is having different behaviors for known and unknown (extensions) attributes correct?

[pierDipi]

/cc @vaikas @lionelvillard @n3wscott

[pierDipi]

Ping.

[pierDipi]

I would like to have an answer to this question before:

Is having different behaviors for known and unknown (extensions) attributes when an empty string is specified correct?

[pierDipi]

These tests don't pass, so:

• is it a bug? or
• do we need to clarify the spec?

[slinkydeveloper]

Is having different behaviors for known and unknown (extensions) attributes when an empty string is specified correct?

Can you clarify?

[slinkydeveloper]

Ah i see, what happens when the extension doesn't exist at all in the event but there is a specific filter with an empty string, like extname1: "", am i right? in this case, i suppose the event should pass

[pierDipi]

Yes, you're right.

So, if I insert an extension that doesn't exist in the event, should the event pass?

• If yes, it's a bug
• if no, should we apply the same semantic to known and optional attributes like subject?
  • if yes, they both shouldn't pass, so it's a bug.
  • if no, we need to clarify this in the spec.

[slinkydeveloper]

@pierDipi Can you bring this issue to the next week eventing delivery WG? At least add it to the calendar, if you can't be there, i can take care of it

[pierDipi]

Sure

[knative-prow-robot]

@pierDipi: PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[github-actions]

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Reopen with /reopen. Mark as fresh by adding the
comment /remove-lifecycle stale.

[pierDipi]

Issue: #4847
/close

[knative-prow-robot]

@pierDipi: Closed this PR.

Details

In response to this:

Issue: #4847
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.