Account for missing fields in Rollout HealthStatus #1699
Conversation
| @@ -1,4 +1,8 @@ | |||
| tests: | |||
| - healthStatus: | |||
There was a problem hiding this comment.
Am I an idiot - is this the test?
There was a problem hiding this comment.
Yes this is the test. We have that documented here: https://argoproj.github.io/argo-cd/operator-manual/health/#way-2-contribute-a-custom-health-check but I can see how that can be confusing. Honestly, I think we may want to rethink this testing framework. The current tests are valuable but I found it difficult to maintain.
There was a problem hiding this comment.
cool - you still need to sync with master
|
Can this be simplified if we drop v0.2 rollout support? |
|
@jessesuen it would simplify the overall script by a few lines but that would be unrelated to the change i made. A lot of the complexity comes from trying to provide a message of the state of the rollout in the health status. |
dthomson25
force-pushed
the
fix-rollout-status
branch
from
86c616c
to
5ce2e71
Compare
Codecov Report
@@ Coverage Diff @@
## master #1699 +/- ##
==========================================
- Coverage 33.98% 33.96% -0.02%
==========================================
Files 75 75
Lines 11344 11333 -11
==========================================
- Hits 3855 3849 -6
+ Misses 6957 6952 -5
Partials 532 532
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #1699 +/- ##
==========================================
- Coverage 33.98% 33.96% -0.02%
==========================================
Files 75 75
Lines 11344 11333 -11
==========================================
- Hits 3855 3849 -6
+ Misses 6957 6952 -5
Partials 532 532
Continue to review full report at Codecov.
|
|
You've got Jesse's approval - so you can dismis mine and merge. |
Address #1697