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

Allowed cloud network without a zone should return everything #207

Merged
merged 1 commit into from
Feb 12, 2018

Conversation

d-m-u
Copy link
Contributor

@d-m-u d-m-u commented Feb 9, 2018

This was originally merged erroneously because Travis run didn't complete all tests on #199. If it had, I think it would've failed due to the error this fixes. It passed originally because the call to all_cloud_networks was wrong and got fixed here: https://github.com/ManageIQ/manageiq/pull/16824/files. Before the last PR got merged, this test would've been correct as the call erroneously returned nothing. Since the merge of ManageIQ/manageiq#16824, the call for azure returns everything, and thus this test needs to change.

Should return all, not none, due to https://github.com/d-m-u/manageiq/blob/59ba714ccb71b2eb3cdd46157587e85329af4f3b/app/models/manageiq/providers/cloud_manager/provision_workflow.rb#L98.

related bzs:

Fixes issue caused by fix of https://bugzilla.redhat.com/show_bug.cgi?id=1533277
(Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1535189)

This was originally merged erroneously because Travis run didn't complete all tests
@d-m-u
Copy link
Contributor Author

d-m-u commented Feb 9, 2018

@miq-bot add_label test
@syncrou

@miq-bot miq-bot added the test label Feb 9, 2018
@d-m-u
Copy link
Contributor Author

d-m-u commented Feb 9, 2018

@miq-bot assign @bronaghs
@Ladas can you review please?

@miq-bot
Copy link
Member

miq-bot commented Feb 9, 2018

Checked commit d-m-u@759b8f9 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 👍

@gmcculloug
Copy link
Member

Discussed with @d-m-u and identified the reason the test was passing initially. (Now documented in the PR description).

@gmcculloug gmcculloug merged commit 4df4696 into ManageIQ:master Feb 12, 2018
@gmcculloug gmcculloug added this to the Sprint 79 Ending Feb 12, 2018 milestone Feb 12, 2018
@d-m-u d-m-u deleted the fixing_merged_cloud_network_test branch February 12, 2018 14:06
@d-m-u
Copy link
Contributor Author

d-m-u commented Apr 2, 2018

@miq-bot add_label gaprindashvili/yes

simaishi pushed a commit that referenced this pull request Apr 2, 2018
Allowed cloud network without a zone should return everything
(cherry picked from commit 4df4696)
@simaishi
Copy link
Contributor

simaishi commented Apr 2, 2018

Gaprindashvili backport details:

$ git log -1
commit 6b255d811bec27b1b3e86a3357eb1b3b6a9d76d0
Author: Greg McCullough <[email protected]>
Date:   Mon Feb 12 09:05:25 2018 -0500

    Merge pull request #207 from d-m-u/fixing_merged_cloud_network_test
    
    Allowed cloud network without a zone should return everything
    (cherry picked from commit 4df4696efbc2cd28e14e95508c3509eacb3c77ae)

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.

5 participants