Skip to content
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

Tests for parse_pod, specifically for STI type if missing pod status #194

Merged
merged 1 commit into from
Jan 8, 2018

Conversation

cben
Copy link
Contributor

@cben cben commented Dec 27, 2017

Followup to #177. As discussed there, testing actual type set in DB in refresher_spec might be best, but it turned out harder than I thought to capture pods without status in VCR. A pod with bad image does get a status, with waiting state. (Maybe requesting resources above quota to keep it pending would work?)

Instead, I captured various states a pod goes through from oc get pods --watch --show-all --all-namespaces -o json into refresh_parser_spec.
(Tip: jq 'select((.spec.containers | length) > (.status.containerStatuses | length))' finds states with missing statuses.)

We already have some tests for parse_container_status/state, but the bug here

Wasn't sure what to test, for now included all states but only tested STI types.

Verified test failed before #177:

parse_pod
  sets correct STI types for container_creating_pod
  sets correct STI types for just_created_pod (FAILED - 1)
  sets correct STI types for graceful_deletion_pod
  sets correct STI types for deleted_pod
  sets correct STI types for scheduled_pod (FAILED - 2)
  sets correct STI types for err_pod

https://bugzilla.redhat.com/show_bug.cgi?id=1517676
@miq-bot add-labels inventory, test, gaprindashvili/yes

@zeari @enoodle @moolitayer please review
(not cc'ing Ladas because holidays)

@miq-bot
Copy link
Member

miq-bot commented Dec 27, 2017

Checked commit cben@1c8e2a6 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 👍

Copy link
Member

@yaacov yaacov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@Ladas Ladas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great 👍

Copy link

@moolitayer moolitayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@moolitayer moolitayer merged commit 2df4967 into ManageIQ:master Jan 8, 2018
@moolitayer moolitayer added this to the Sprint 77 Ending Jan 15, 2018 milestone Jan 8, 2018
simaishi pushed a commit that referenced this pull request Jan 8, 2018
Tests for parse_pod, specifically for STI type if missing pod status
(cherry picked from commit 2df4967)

https://bugzilla.redhat.com/show_bug.cgi?id=1524630
@simaishi
Copy link

simaishi commented Jan 8, 2018

Gaprindashvili backport details:

$ git log -1
commit 71cefb8eef9eac57a705826d3d77148542e2c407
Author: Mooli Tayer <[email protected]>
Date:   Mon Jan 8 11:13:17 2018 +0200

    Merge pull request #194 from cben/test_parse_pod
    
    Tests for parse_pod, specifically for STI type if missing pod status
    (cherry picked from commit 2df4967cba252e47c50c5388944f0953b1a6e667)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1524630

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants