search

thoth-station / integration-tests

Integration tests for the Thoth project to make sure deployment works as expected
GNU General Public License v3.0
4 stars 12 forks source link

Need a test for `thamos images` #348

Closed codificat closed 1 year ago

codificat commented 1 year ago

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

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

VannTen commented 1 year ago

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 ?

codificat commented 1 year ago

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?

VannTen commented 1 year ago

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

goern commented 1 year ago

/assign

goern commented 1 year ago

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

@VannTen @codificat

VannTen commented 1 year ago

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.

codificat commented 1 year ago

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

Gkrumbach07 commented 1 year ago

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

sesheta commented 1 year ago

@Gkrumbach07: Closing this issue.

In response to [this](https://github.com/thoth-station/integration-tests/issues/348#issuecomment-1405651393): >Closing as parent [issue](https://github.com/thoth-station/thamos/issues/1184) 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](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.