-
Notifications
You must be signed in to change notification settings - Fork 229
improve: showcase issue where updating the spec with SSA removes the finalizer #2848
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what the purpose of this test should be? More precisely, I'm not sure we should add showcases to the test suite, such examples are probably better be put in the samples, rather as it's not testing anything in our implementation and just adds to the test running time.
fair point, yes, it definitely adds to test running time, although these tests are relatively fast. We already have integration tests which are just showing some issue, or some corner cases. My goal was with these to add them to the test suit, so I can reference it from an upcomin blogpost, what will be also published from JOSDK repo. Regarding the runtime of these integration tests is now something horrible, for a full e2e / IT test suit is ok to have a longer running time. |
e694035
to
a3f4acf
Compare
…nalizer if the finalizer is not explicitly added to the fresh resource Signed-off-by: Attila Mészáros <[email protected]>
Signed-off-by: Attila Mészáros <[email protected]>
a3f4acf
to
5eee1ba
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @metacosm that we might consider spliting samples (especially if we want to add more of them) into a separate repo. We can set up GH actions to maintain them as if they would be in this repo.
...k/src/test/java/io/javaoperatorsdk/operator/baseapi/ssaissue/specupdate/SSASpecUpdateIT.java
Outdated
Show resolved
Hide resolved
...k/src/test/java/io/javaoperatorsdk/operator/baseapi/ssaissue/specupdate/SSASpecUpdateIT.java
Show resolved
Hide resolved
Signed-off-by: Attila Mészáros <[email protected]>
The only objection to that, is it would be much harder to maintain an index, since we cannot use the same approach as here: Note that we could also have these in a separate package and run them with a separate GitHub action, so it does not add to the time of the standard integration test suite. cc @metacosm what do you think about that? I generally find having it in a separate repo as an unnecessary maintenance burden, for example, we would have to bump the josdk version on every release in that repo, also to think about that every time we change something. If we run these as a parallel GitHub action, basically, it has no downside, at least not what I can see. I agree that these tests are a bit different than the current integration tests, but those already serves dual purpose: |
No description provided.