search

Kuadrant / testsuite

3 stars 13 forks source link

Add blame to metadata tests #400

Closed pehala closed 3 months ago

ficap commented 4 months ago

AFAIK rhsso.client.create_uma_resource("get1", ["/anything"]) and the one below should be also blamed

trepel commented 3 months ago

AFAIK rhsso.client.create_uma_resource("get1", ["/anything"]) and the one below should be also blamed

Should not https://github.com/Kuadrant/testsuite/pull/400/files#diff-afa1a2b71e5e62d18ea25af8e2fa2d50c16bad28fad39df4e03f3d9a573be8e7L127 be blamed as well then?

I wanted to review this PR but I realized that I don't really know what should be blamed and what not. If somebody feels like explaining that to me that would be great.

pehala commented 3 months ago

It most likely should, tests share one rhsso realm for the entirety of the test run, so every rhsso resourxe should ideally be blamed to prevent collisions. I changed only a few instances of this bevausw it came up as a actual collisin but there are probably more instances of this scattered among the tests

trepel commented 3 months ago

Thanks! So blaming is primarily used for Keycloak resources to avoid potential collisions. With this knowledge I re-reviewed and I think that after blaming the protected resource name as I suggested this is good to go. To be super fair the test creates one more Keycloak resource - Scope called get - but the name is derived from HTTP method and is expected to be that way, blaming that would be a bit more involved so I would leave it out of scope of this PR.