Update e2e test data for test-framework #2129
Conversation
openshift-ci-robot
added
the
size/M
camilamacedo86
changed the title
test/test-framework/pkg/apis/cache/v1alpha1/memcachedrs_types.go
Outdated
Show resolved
Hide resolved
test/test-framework/pkg/controller/memcached/memcached_controller.go
Outdated
Show resolved
Hide resolved
camilamacedo86
changed the title
openshift-ci-robot
added
size/L
There was a problem hiding this comment.
LGTM
Can you also please update the PR title and commit subject when you squash to be something like Update e2e test data for test-framework since we're not updating the test-framework itself.
You can add more details in the commit message body if you'd like.
camilamacedo86
changed the title
camilamacedo86
added
approved
| @@ -0,0 +1,221 @@ | |||
| // +build !ignore_autogenerated | |||
There was a problem hiding this comment.
Can you remove this file? The SDK doesn't use this generated code.
Side note: I'm considering removing the generator for this file, since most SDK users do not use it, or at least make it optional.
There was a problem hiding this comment.
HI @estroz the update of the project has been done using the SDK so it will be generated since its commands generate it. Note that the goal is also to add a check to ensure that the mock data is updated with the current code source implementation always to avoid we are testing it with something that should be changed already. See #2131 for we add the test in the sanity.
So, if in the future we remove this file and/or change the commands then the test-framework should be updated accordingly to pass in the CI. Is it make sense?
PS.: I am trying here give the first steps for we stop to do changes manually on it regards the gen artefacts and in this avoid unexpected scenarios as well.
There was a problem hiding this comment.
For now we can keep it here. If we decide that removing the openapi generator is the right way to go we can remove this file.
|
NOTE: The sanity is falling because of an issue with the master. I just rebased it with and the issue appears. It will be solved here: #2169 |
Description of the change:
Motivation for the change:
Follow up of : #2130
Related to: #2114 and #2042