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

update Apache image stream naming, Apache QuickStart, and add Ruby 2.… #16214

Merged
merged 1 commit into from
Sep 9, 2017
Merged

update Apache image stream naming, Apache QuickStart, and add Ruby 2.… #16214

merged 1 commit into from
Sep 9, 2017

Conversation

coreydaley
Copy link
Member

…4 image stream

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 7, 2017
@coreydaley
Copy link
Member Author

@luciddreamz please verify the changes you requested.

@openshift/devex ptal

@gabemontero
Copy link
Contributor

/approve

@csrwng
Copy link
Contributor

csrwng commented Sep 7, 2017

@coreydaley looks like you need to fix these tests:
https://github.com/openshift/origin/blob/master/test/cmd/newapp.sh#L241-L242

@coreydaley
Copy link
Member Author

@csrwng updated, thanks!

@coreydaley
Copy link
Member Author

/retest

@csrwng
Copy link
Contributor

csrwng commented Sep 7, 2017

@coreydaley more tests to fix :)
https://github.com/openshift/origin/blob/master/test/cmd/newapp.sh#L294

@coreydaley
Copy link
Member Author

update pushed!

@bparees
Copy link
Contributor

bparees commented Sep 7, 2017

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 7, 2017
@openshift-merge-robot openshift-merge-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 7, 2017
@coreydaley
Copy link
Member Author

/retest

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@bparees
Copy link
Contributor

bparees commented Sep 8, 2017 

=== BEGIN TEST CASE ===
test/cmd/newapp.sh:364: executing 'oc new-app --image-stream ruby https://github.com/openshift/rails-ex --dry-run' expecting failure and text 'error: only a partial match was found for \"ruby\":'
FAILURE after 1.033s: test/cmd/newapp.sh:364: executing 'oc new-app --image-stream ruby https://github.com/openshift/rails-ex --dry-run' expecting failure and text 'error: only a partial match was found for \"ruby\":': the output content test failed
There was no output from the command.
Standard error from the command:
error: multiple images or templates matched "ruby": 2

The argument "ruby" could apply to the following Docker images, OpenShift image streams, or templates:

* Image stream "ruby" (tag "2.4") in project "cmd-newapp"
  Use --image-stream="cmd-newapp/ruby:2.4" to specify this image or template

* Image stream "ruby" (tag "2.3") in project "cmd-newapp"
  Use --image-stream="cmd-newapp/ruby:2.3" to specify this image or template
=== END TEST CASE ===

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot removed the lgtm Indicates that a PR is ready to be merged. label Sep 8, 2017
bparees
bparees requested changes Sep 8, 2017
View reviewed changes
test/cmd/newapp.sh Outdated
@@ -361,6 +361,7 @@ os::cmd::expect_success 'oc delete imagestreamtag ruby:latest'
os::cmd::expect_failure_and_text 'oc new-app --image-stream ruby https://github.com/openshift/rails-ex --dry-run' 'error: multiple images or templates matched \"ruby\":'
# when only 2.3 exists, there is a single partial match (2.3)
os::cmd::expect_success 'oc delete imagestreamtag ruby:2.2'
os::cmd:;expect_success 'oc delete imagestreamtag ruby:2.4'
Copy link
Contributor

Choose a reason for hiding this comment

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

if you want to keep this consistent, you should be deleting 2.2 and 2.3 (so that the only tag left is the 2.4 tag which is also referenced by the "latest" tag).

of course you'll also have to update the next test that expects success with "2.3" to instead expect success with "2.4"

Copy link
Member Author

Choose a reason for hiding this comment

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

updated

@bparees
Copy link
Contributor

bparees commented Sep 8, 2017

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 8, 2017
@openshift-merge-robot openshift-merge-robot removed the lgtm Indicates that a PR is ready to be merged. label Sep 8, 2017
@bparees
Copy link
Contributor

bparees commented Sep 8, 2017

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 8, 2017
@openshift-merge-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bparees, coreydaley, gabemontero

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@coreydaley
Copy link
Member Author

/retest

@coreydaley
Copy link
Member Author

/retest

@bparees
Copy link
Contributor

bparees commented Sep 9, 2017 via email

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@bparees
Copy link
Contributor

bparees commented Sep 9, 2017 via email

@0xmichalis
Copy link
Contributor

/retest

@openshift-merge-robot
Copy link
Contributor

/test all [submit-queue is verifying that this PR is safe to merge]

@openshift-merge-robot
Copy link
Contributor

Automatic merge from submit-queue (batch tested with PRs 16258, 16214)

@openshift-merge-robot openshift-merge-robot merged commit f51e6a8 into openshift:master Sep 9, 2017
@openshift-ci-robot
Copy link

openshift-ci-robot commented Sep 9, 2017 •
edited
Loading

@coreydaley: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/openshift-jenkins/unit d5ea59b link /test unit

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@luciddreamz luciddreamz luciddreamz approved these changes

@bparees bparees bparees approved these changes

Assignees

@bparees bparees

@csrwng csrwng

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@coreydaley @gabemontero @csrwng @bparees @openshift-bot @openshift-merge-robot @0xmichalis @openshift-ci-robot @luciddreamz