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

remove test file cruft #7993

Merged

Conversation

stevekuznetsov
Copy link
Contributor

@smarterclayton the changes in test/cmd/admin remove your previous comments

@fabianofranz the changes in test/cmd/help get rid of tests for openshift cli help which looks to have been deprecated?

@smarterclayton
Copy link
Contributor

Hrm - that todo for test/cmd/help implies the tests should be fixed, not removed.

The manage-node stuff needs to be moved to extended. Can you create an issue and paste them in as "need unit tests for manage-node"?

@stevekuznetsov
Copy link
Contributor Author

@smarterclayton issue for manage-node here: #7997

@stevekuznetsov
Copy link
Contributor Author

@fabianofranz re: openshift cli help - the intent here is to have a test that ensures that openshift cli help (and oc help?) are wired up and working, right? we aren't explicitly trying to test the help output of any command, are we? Would one call to openshift cli help suffice?

@fabianofranz
Copy link
Member

@stevekuznetsov Right, help being wired up and spitting the output expected for it (e.g. we get the help for the expected command instead of main help). Not testing any particular command though, no.

So I agree we don't need all of them, this could be reduced to just a couple tests - one call to openshift cli help and one to one of those commands, whatever it is, checking part of the output.

Related issue is #6042.

@stevekuznetsov
Copy link
Contributor Author

Oh, I had created an issue when I removed those tests. OK. If an issue exists to fix both of these commented-out test cases, we should be good to remove the lines for now, right?

@stevekuznetsov
Copy link
Contributor Author

@smarterclayton would like your opinion - if issues (#6042 and #7997) exist for both of these cases, is this ready to merge?

@smarterclayton
Copy link
Contributor

Yes

@stevekuznetsov
Copy link
Contributor Author

@smarterclayton could you please then slap a merge tag on this

@smarterclayton
Copy link
Contributor

[merge]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to a18cfc9

@openshift-bot
Copy link
Contributor

[Test]ing while waiting on the merge queue

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to a18cfc9

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/2587/)

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/2587/) (Image: devenv-rhel7_3871)

@stevekuznetsov
Copy link
Contributor Author

Jenkins flaked on UI e2e breadcrumb here:


**************************************************
*                    Failures                    *
**************************************************

1)  authenticated e2e-user new project when creating a new project should successfully create a new project
  - Error: Element not found: .breadcrumb li a

#6618
@smarterclayton please re-tag for merge

@openshift-bot openshift-bot merged commit 2c74733 into openshift:master Mar 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants