Need a test for `thamos images`

Question

Need a test for `thamos images`

Closed this issue 2 years ago · 9 comments

thamos images stopped working correctly after a change in user-api (thoth-station/thamos#1184) and this is not captured by current integration tests.

The current integration tests include containe images API coverage but not from thamos.

goern commented 2 years ago

/assign

Answer 1 · 2022-10-11T10:22:05.000Z

Could'nt we have an automatic PR generating the thamos openapi client when the relevant user-api is updated, and let the usual ci (mypy in particular for catching bad usages) point us to fixes ?

Answer 2 · 2022-10-11T11:14:33.000Z

Could'nt we have an automatic PR generating the thamos openapi client when the relevant user-api is updated

That would be nice to have, but we don't have anything similar available yet, right?

and let the usual ci (mypy in particular for catching bad usages) point us to fixes ?

Would mypy catch this particular issue though? I think the problem in the code involved in thoth-station/thamos#1184 is more semantic/logic than what mypy would catch, no?

Answer 3 · 2022-10-11T12:37:12.000Z

On Tue, Oct 11, 2022 at 04:14:45AM -0700, Pep Turró Mauri wrote: > and let the usual ci (mypy in particular for catching bad usages) point us to fixes ? Would mypy catch this particular issue though? I think the problem in [the code involved](https://github.com/thoth-station/thamos/blob/4536f4d67e362107a2d40ceaef34901365148a1c/thamos/lib.py#L1316-L1329) in thoth-station/thamos#1184 is more semantic/logic than what mypy would catch, no?

I don't know swapper-codegen enough, but in principle it's possible to represent a 404 as Optionnal/Union/ that sort of things (maybe I'm a bit optimistic on the general support of typing across the Python ecosystem ^). Missing that, just opening a PR automatically would still be good: during review, we would have the opportunity to spot unhandled API responses, etc.

Answer 4 · 2022-10-19T10:56:47.000Z

do we turn this into a RFE (wrt openapi regeneration...) and close this because auf #351 ?

@VannTen @codificat

Answer 5 · 2022-10-19T11:41:44.000Z

It might be in a separate issue. I think a integration test for thamos images is easier than automatic openapi regeneration, and it can probably catch other things.

Answer 6 · 2022-11-16T12:39:13.000Z

/sig stack-guidance
/kind feature
/priority important-longterm

Answer 7 · 2023-01-26T21:06:34.000Z

Closing as parent issue is resolved and these test will no longer be relevant. As long as /container-images is tested, then this thamos command will be covered as well because the parent issue removed any additional logic that was applied after the api call.

/close

Answer 8 · 2023-01-26T21:06:37.000Z

@Gkrumbach07: Closing this issue.

In response to this:

Closing as parent issue is resolved and these test will no longer be relevant. As long as /container-images is tested, then this thamos command will be covered as well because the parent issue removed any additional logic that was applied after the api call.

/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.