Please tell me this makes more sense, I've spent too much time refactoring this mess. :)

we should have available the object "current_app", why you didn't use it?

What if someone gives you "testcase" == None or "testcase" == "" ?
It will give you True... why did you put "not testcase"? It's not clear to me

What about the "matches" method in the "PackageSpecificRule"? It's inherited from "Rule"... but it returns always "True"... shoudn't we check if it actually matches?

we should have available the object "current_app", why you didn't use it?

Is there difference? (I would rather avoid touching some global state.)

I don't think that testcase with empty name is valid so I just ignore it here, i.e. treat it as "match rule with any test case name".

Let me know if it doesn't make sense and why.

What about the "matches" method in the "PackageSpecificRule"? It's inherited from "Rule"... but it returns always "True"... shoudn't we check if it actually matches?

Fixed. I also added the missing tests.

No difference, but at this point, let's change it everywhere. It doesn't make sense to use one in one place and another one in another place

No difference, but at this point, let's change it everywhere. It doesn't make sense to use one in one place and another one in another place

I only add self.flask_app in this patch not current_app. If it's about getting rid of self.flask_app it should be separate change.

Is this now done somewhere else?

Thank you for this change. It does enhance the code quality significantly IMO.

Ah, forgot about this. I don't think this makes any sense after removal of greenwave.resources.retrieve_update_for_build() (8658b9d5d98f0ed5df4457fe859cfed492380ef1). Now it would use the original test subject ID with incorrect type (koji_build instead bodhi_update). I think that always leads to "missing test result" response.

I think it should be fine.
+1 for me

:+1:

Merged. I may also found a bug that this PR should fix...