Add tests for new operator-sdk related code #122
Conversation
|
This change is |
|
You'll note that I'm using "strategy" and "strategy chooser" in the test already, even though I haven't moved the old controllers to this new name yet. This is to keep the PR small and focused, and I'll send another PR with the renaming. |
|
e2e tests are passing: |
Signed-off-by: Juraci Paixão Kröhling <[email protected]>
jpkrohling
force-pushed
the
119-Add-tests-to-new-controller
branch
from
f355798 to
6f8ed3d
Compare
Codecov Report
@@ Coverage Diff @@
## master #122 +/- ##
==========================================
+ Coverage 92.12% 95.64% +3.51%
==========================================
Files 27 27
Lines 1143 1148 +5
==========================================
+ Hits 1053 1098 +45
+ Misses 86 38 -48
- Partials 4 12 +8
Continue to review full report at Codecov.
|
| ) | ||
|
|
||
| type fakeStrategy struct { | ||
| dependencies func() []batchv1.Job |
There was a problem hiding this comment.
Not related to this PR, but just wondering if dependencies should be strictly typed to Job instead of also runtime.Object - to cover examples that we discussed about setting up configmaps as dependencies? Just raised here as a note - if relevant I will raise as separate issue.
There was a problem hiding this comment.
For now, I think it should. We talked about routes/config maps being dependencies, but they are usually available instantly. The assumption is then that bigger tasks would be wrapped within a batch job, which provides a "succeed" flag, indicating whether or not the dependency processing finished executing. The batch job could, for instance, deploy another operator and wait for a CR to be ready and available.
Signed-off-by: Juraci Paixão Kröhling <[email protected]>
|
PR updated. |
There was a problem hiding this comment.
Reviewed 7 of 8 files at r1, 1 of 1 files at r2.
Reviewable status:complete! all files reviewed, all discussions resolved
Closes #119
Signed-off-by: Juraci Paixão Kröhling [email protected]