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.

Sign up for GitHub

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

Jump to bottom

JBIDE-23463 Properties: Build pod has state of exit code 0 instead of correct status completed #238

Merged
merged 2 commits into from Nov 10, 2016
Merged

JBIDE-23463 Properties: Build pod has state of exit code 0 instead of correct status completed #238

merged 2 commits into from Nov 10, 2016

Conversation

scabanovich
Copy link
Contributor

Logic of Pod.getStatus() is fixed to copy that of
resources.js .filter('podStatus')

@scabanovich
JBIDE-21463 Properties: Build pod has state of exit code 0 instead of…
7eebc6a 
… correct status completed

Logic of Pod.getStatus() is fixed to copy that of
resources.js .filter('podStatus')
@jcantrill
Copy link
Contributor

Looks good. Are there tests to be added as well? [test]

@openshift-bot
Copy link
Contributor

Evaluated for javaclient test up to 7eebc6a

@openshift-bot
Copy link
Contributor

Java Client Test Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test-openshift-restclient-java/292/) (Base Commit: 76a5ca4)

@adietish adietish changed the title JBIDE-21463 Properties: Build pod has state of exit code 0 instead of correct status completed JBIDE-23463 Properties: Build pod has state of exit code 0 instead of correct status completed Nov 9, 2016
@adietish
Copy link
Member

adietish commented Nov 9, 2016

@scabanovich thx for fixing this!
You might want to correct the commit msg: JBIDE-21463 -> JBIDE-23463
And yes, it would be even greater if you could add tests. PodTest currently passes while it shouldnt, since we consider the current behavior as faulty. So there's at least a missing assertion for the current faulty behavior

@scabanovich
Copy link
Contributor Author

I have added some tests.

jcantrill
jcantrill reviewed Nov 10, 2016
View reviewed changes
src/test/java/com/openshift/internal/restclient/model/v1/PodTest.java
*/
@Test
public void testGetStatusTerminateReasonAndExit() {
ModelNode node = new ModelNode();
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM. In future, there is a ModelNodeBuilder class that might make it easier for your to build up the json.

@jcantrill
Copy link
Contributor

[merge]

@openshift-bot
Copy link
Contributor

Evaluated for javaclient merge up to 2c1584f

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 10, 2016 •
edited

Java Client Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test-openshift-restclient-java/294/) (Base Commit: 461469e)

@openshift-bot openshift-bot merged commit 4a2db9b into openshift:master Nov 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jcantrill jcantrill jcantrill left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@scabanovich @jcantrill @openshift-bot @adietish