eventing icon indicating copy to clipboard operation
eventing copied to clipboard

Merge TLS and OIDC e2e tests

Open creydr opened this issue 1 year ago • 7 comments

Currently the TLS and OIDC e2e tests are mostly redundant and test the same (e.g. both have test to test the reply & DLS). Mostly the only difference is, that the OIDC tests configure an audience.

Therefor we should check, which OIDC and TLS tests can be merged and merge both "suites" together.

Hint:

  • Some tests still might be only relevant for TLS or OIDC (e.g. TestBrokerSupportsOIDC which tests only for OIDC compliance)

creydr avatar Jan 29 '24 10:01 creydr

Hi @creydr, I am interested in this issue and want to gain more experience in this topic as a beginner.

parth721 avatar Feb 18 '24 15:02 parth721

Hello @parth721, thanks for showing interest in this issue :+1: Anyhow, in case you're new to knative or the code base, I think a good-first-issue could be a good start to get into the code bases and the development process. You can check for good-first-issues e.g. here: https://github.com/search?q=org%3Aknative+org%3Aknative-extensions++label%3A%22good+first+issue%22+is%3Aissue+is%3Aopen+&type=issues. In case you want to "prepare" for these issues, check for TLS or OIDC related issues.

Anyhow if you feel confident for this issue already, feel free to assign it to you.

creydr avatar Feb 19 '24 09:02 creydr

Hey @creydr, you are right, picking a goodfirst issues to understand the codebase is a great idea, but the catch is that all the issues are assigned, and some issues are merged anywhere else but kept open as goood first issue( 1yr old). Are these issues still kept opened for beginners or is there any labels similar to good first issue(level 2) :). what about difficulty level of flaky issues ? I don't know much about them, but i'm happy to learn.

parth721 avatar Feb 19 '24 15:02 parth721

Hey @creydr, I was going through the tests yesterday and noticed that TestMTChannelBrokerRotateTLSCertificates and TestBrokerSupportsOIDC from broker_test.go, as well as (to some extent) TestInMemoryChannelTLS and TestChannelImplSupportsOIDC from channel_test.go, have a significant overlap in environment setup and focus on similar functionalities. I think merging these tests into single functions with clear subtests would indeed streamline our test suite... I'm looking to fix this issue and wanted to confirm if my identification is correct or if there are some other aspects left to consider.

Thanks!

prajjwalyd avatar Jun 13 '24 15:06 prajjwalyd