Add FBT for replication controller list #401
Conversation
Current coverage is
|
|
@cheld I will review |
|
|
||
| | Factor | 1 | 2 | 3 | 4 | 5 | 6 | Comment | | ||
| |--------------------------------------|--------------------------------------------------|-----------------------------------------------|------------------------------------------|-----------------------------------------|-------------------------------------------------------------------|---------------------------------------------------------------------|---------| | ||
| | Replication controller count on page | 1 replication controller on wide page | 1 replication controller on narrow page | 3 replication controller on on wide page | 3 replication controller on narrow page | Any other positive number of replication controllers on wide page | Any other positive number of replication controllers on narrow page | | |
There was a problem hiding this comment.
Layout | normal | mobile | narrow | wide | active resizing
RC count | 0 (?) | 1 | odd | even | 20-30
There was a problem hiding this comment.
I would suggest to keep layout and rc count separated as two factors. Easy for tester to combine
|
I gave some comments to give you some ideas. I would shorten the text - unless it is not understandable. I am not sure if we always should state normal cases like '1', 'short', 'ok'. Maybe we should remove them. They are obvious anyway. We add them just for completeness. But I am on the fence and undecided. |
|
I am not sure about concurrency. Maybe we should de-normalize and add it as a specific factor. Delete concurrently, scale pods up & down concurrently... I think we should do that otherwise it is forgotten |
|
BTW: found 3 bugs during review :) |
|
@cheld Thank you for review, I've applied modifications. PTAL |
| | Replication controller labels | normal | long text (253 prefix and 63 label) | 1 | odd | even | short and long text mixed | 20-30 | | labels increase height of the card | | ||
| | Actions | view details | edit pod count | delete | display logs | | | | | | | ||
| | Edit pod count | negative | 0 | 1 | less than 10 | more than 30 | empty | scale down | cancel | | | ||
| | Delete replication controller action | normal | with checkbox checked | cancel | | | | | | | |
There was a problem hiding this comment.
I don't know the algorithm in details, but maybe a deployment is possible (using kubectl) that will produce something like the following:
- the wrong service is deleted
- no service is deleted
- too much is deleted
- error
- nothing happens although the service should be deleted
IMHO, the factor does not encourage deep enough testing. Where should be put this tests? How to describe the factors?
BTW: The checkbox is also shown if no service exits - I would say this is a bug.
There was a problem hiding this comment.
Yes, but if we don't want to combine factors then there are only two possibilities - checked or not. We must remember, that this factor will be combined with other ones. There is a possibility of adding factor telling if replication controllers have services or not, but it's not visible from this view... It's very problematic from my point of view.
There was a problem hiding this comment.
Ok, let's postpone it. We can add it later and check if there is a better place.
|
One last comment. Otherwise, I like it |
|
LGTM |
|
@bryk PTAL |
|
Reviewed 1 of 1 files at r2. docs/test/replication-controller-list.md, line 25 [r2] (raw file): Comments from the review on Reviewable.io |
|
@bryk Fixed typos. PTAL |
|
LGTM Reviewed 1 of 1 files at r3. Comments from the review on Reviewable.io |
Add FBT for replication controller list
Added FBT for replication controller list page. Connected to #350 issue.