fix: allow keeping deleted CRDs in test with configuration #2687
Conversation
...work-junit5/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java
Outdated
Show resolved
Hide resolved
| @@ -43,6 +43,8 @@ public class LocallyRunOperatorExtension extends AbstractOperatorExtension { | |||
|
|
|||
| private static final Logger LOGGER = LoggerFactory.getLogger(LocallyRunOperatorExtension.class); | |||
| private static final int CRD_DELETE_TIMEOUT = 1000; | |||
| private static final LinkedList<AppliedCRD> appliedCRDs = new LinkedList<>(); | |||
| private static final boolean deleteCRDs = Boolean.parseBoolean(System.getProperty("testsuite.deleteCRDs", "true")); | |||
There was a problem hiding this comment.
Shouldn't be these rather here as other configs?
Config
There was a problem hiding this comment.
I don't think we want to configure this per Test, but rather for the whole testsuite. I think that per test it can be confusing.
There was a problem hiding this comment.
Hmm, good point,
I'm working this this now:
#2688
And here the CRD's will have to be managed manually, but fur sure be at least one step where the CRD is updated. But that should be fine too, so yes, if we see that this canont cover such use cases when in particular tests user wants to manage use cases we can still add a property also to builder.
|
@csviri refactored it to ArrayDeque. I think we can merge this. |
|
@xstefank This is just test, it has zero implications what data structure we use there, there fore in these places I usually prefer using the well known data structures like ArrayList. First time somebody going to read that part of "code will ask the question why is there a Dequeu, why is that so specific?". (So might be in this case also reason to make a comment about it). But fine by me. let's wait until @metacosm approves it too, then we can merge. The agreement is both of us approve PRs (except documentation updates). |
|
just changed to LIFO as it makes more sense IMO. |
If we would like to be absolutely correct, Set should be used, to indicate that the order does not matter. IMHO. |
Signed-off-by: xstefank <[email protected]>
|
and it is Set :) |
| @@ -8,11 +8,16 @@ | |||
| import java.nio.file.Files; | |||
| import java.nio.file.Path; | |||
| import java.time.Duration; | |||
| import java.util.ArrayDeque; | |||
There was a problem hiding this comment.
Seems like imports need to be optimized…
No description provided.